8000 Draft: tinyusb cdc class in python / dupterm by andrewleech · Pull Request #16017 · micropython/micropython · GitHub
[go: up one dir, main page]

Skip to content

Draft: tinyusb cdc class in python / dupterm #16017

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

andrewleech
Copy link
Contributor

Summary

In the stm port the built-in USB CDC / virtual com port is exposed as a class in python. It's also connected to repl by being registered in dupterm by default.

The benefits of this are that at runtime the user can de-register it from dupterm to disable repl in that port and then also use it as a custom stream if desired.

This PR replicates this behaviour on other ports which use TinyUSB.

Testing

This has been tested so far on:

  • Renesas

Trade-offs and Alternatives

This does come at a cost to code size.
Currently it's written such that it dupterm is not enabled the previous behaviour is used, with the class no longer created.

A define MICROPY_HW_UART_REPL is commonly used to specify whether a UART should be connected to repl, I'm considering whether a matching MICROPY_HW_USBD_CDC_REPL define should be added to specify whether the CDC should be registered in dupterm automatically or not.

Then there's also a question about whether a separate define should be added to specify if the class should be created anyway in case people want access to that for custom data.

Copy link
codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.54%. Comparing base (eb45d97) to head (9d8632f).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #16017   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files         169      169           
  Lines       21864    21864           
=======================================
  Hits        21545    21545           
  Misses        319      319           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
github-actions bot commented Oct 15, 2024

Code size report:


@andrewleech andrewleech force-pushed the shared_tinyusb_cdc_class branch from b7850b4 to 57f64c8 Compare October 15, 2024 21:54
@andrewleech andrewleech force-pushed the shared_tinyusb_cdc_class branch 3 times, most recently from d50bff1 to a9099b3 Compare October 27, 2024 03:08
@dpgeorge dpgeorge added the ports Relates to multiple ports, or a new/proposed port label Oct 29, 2024
@andrewleech
Copy link
Contributor Author

I started adding send and recv to match the pyb module, particularly to use these with the test_serial.py script and see how much speed improvement you get vs stdin in particular. While this works, it's really not neccesary when there's already the normal stream operations so I will remove them again - users should be able to do everything needed with the stream api and having these it arguably just confusing and costing extra flash.

pi-anl added 8 commits March 3, 2025 10:22
Signed-off-by: Andrew Leech <andrew@alelec.net>
Signed-off-by: Andrew Leech <andrew@alelec.net>
Signed-off-by: Andrew Leech <andrew@alelec.net>
Signed-off-by: Andrew Leech <andrew@alelec.net>
Signed-off-by: Andrew Leech <andrew@alelec.net>
Signed-off-by: Andrew Leech <andrew@alelec.net>
Signed-off-by: Andrew Leech <andrew@alelec.net>
Signed-off-by: Andrew Leech <andrew@alelec.net>
@andrewleech andrewleech force-pushed the shared_tinyusb_cdc_class branch from a9099b3 to 9d8632f Compare March 2, 2025 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ports Relates to multiple ports, or a new/proposed port
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0