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

rom_funcs results output over USB instead of UART #413

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

camrbuss
Copy link
Contributor

@camrbuss camrbuss commented Aug 7, 2022

  • Added f64 tests for chips with rom_version>1
  • Output results over USB Serial instead of hardware UART

@9names
Copy link
Member

9names commented Aug 7, 2022

I like the idea, but just replacing the UART output with USB-UART output doesn't really improve the situation.
A better option would be to have a optional logger feature in the HAL where you can choose between uart, usb-cdc, rtt or defmt for getting textual output from your program.
That way all the examples can output data in the user's preferred format, and end-users can also opt in to get a simpler debug output.
Also: make it a separate PR to updating the example.

index: usize,
}

// write_str from https://stackoverflow.com/a/39491059
Copy link
Member

Choose a reason for hiding this comment

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

As far as I know, the default license of subscriber content published on stack overflow doesn't allow redistribution under MIT / Apache-2 license.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh my, I didn't realize that. How should I rectify the situation?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This might be a dumb question, but why are we buffering anyway?
Can't you push this straight into the SerialPort?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe usbd_serial implements write_str

Is there another way that I am not seeing?

Copy link
Member

Choose a reason for hiding this comment

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

I was assuming it wouldn't be difficult to build a wrapper struct for usbd_serial and implement write_str on that, but I think the generics would be a pain.
Maybe it would be easier to make a PR to get usbd_serial to (conditionally) impl write_str

Copy link
Member

Choose a reason for hiding this comment

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

Looking at rust-embedded-community/usbd-serial#16, seems this has already been proposed and declined since the blocking call would have to poll the USB device (which it can't).
So: would really require having some logger/stdin+stdout functionality, which would possibly also be part of how we could resolve #456

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.

3 participants