Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command for displaying image streams #11

Merged
merged 15 commits into from
Jul 24, 2024
Merged

Add command for displaying image streams #11

merged 15 commits into from
Jul 24, 2024

Conversation

NicoBiernat
Copy link
Member

Some things that could be improved / changed:

  • use futures instead of polling the stream and key events
  • integrate this into the "cat" command
  • add a border and title around the display

@NicoBiernat NicoBiernat requested a review from fwcd July 18, 2024 21:34
Copy link
Member

@fwcd fwcd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks pretty cool and I look forward to having a way of viewing streamed frames graphically in real-time! I have left some, mostly technical notes, below.

One remark:

integrate this into the "cat" command

That's an interesting idea, but I would probably keep it in a separate command, just to keep things simple and because there doesn't seem to be an obvious Unix analogon to this functionality.

src/cmd/mod.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
src/cmd/display.rs Outdated Show resolved Hide resolved
src/cmd/display.rs Outdated Show resolved Hide resolved
@NicoBiernat
Copy link
Member Author

The application still hangs after a while for unknown reasons...

@fwcd fwcd marked this pull request as ready for review July 24, 2024 01:41
@fwcd
Copy link
Member

fwcd commented Jul 24, 2024

Thanks again!

I have merged the changes from the main branch, which updates lighthouse-client to 3.4.0. In my tests, it seems to run pretty reliably, so hopefully these issues are fixed. If not, feel free to open another issue.

The canvas now uses a custom Shape implementation that makes sure to render the display at an integer scale to make sure that no rows or columns are skipped. Finally, the display now also forwards all key strokes other than q (which quits the command) as input events to the server, enabling games etc.

@fwcd fwcd merged commit 206f1d1 into main Jul 24, 2024
1 check passed
@fwcd fwcd deleted the display branch July 24, 2024 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants