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

boards/esp32s3-pros3: Fix stdio kconfig model #19708

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

MrKevinWeiss
Copy link
Contributor

Contribution description

Always something...

It seems like the desired behaviour (if we use MODULE_USBUS, then override the MODULE_STDIO_USB_SERIAL_JTAG) requires selection of MODULE_USBUS_CDC_ACM. Seems to be a bit of a corner case.

Testing procedure

Let's see if murdock can test faster than my cpu.

Issues/PRs references

Failure in master

@MrKevinWeiss MrKevinWeiss added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 5, 2023
@github-actions github-actions bot added Area: boards Area: Board ports Area: Kconfig Area: Kconfig integration labels Jun 5, 2023
@riot-ci
Copy link

riot-ci commented Jun 5, 2023

Murdock results

✔️ PASSED

c7eadb2 boards/esp32s3-pros3: Fix stdio kconfig model

Success Failures Total Runtime
530 0 530 03m:28s

Artifacts

@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 5, 2023
@bors
Copy link
Contributor

bors bot commented Jun 5, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@MrKevinWeiss
Copy link
Contributor Author

MrKevinWeiss commented Jun 6, 2023

oh man, I forgot to push the enable nightly commit... Should we just move on it and monitor the next nightly. It takes a really long time to verify locally and so far I could not find any issues.

@MrKevinWeiss
Copy link
Contributor Author

local usb based tests and hello world pass...

@@ -25,6 +25,7 @@ config BOARD_ESP32S3_PROS3
select HAS_TINYUSB_DEVICE
# Only this board has a requirement to use USB_BOARD_RESET with STDIO_USB_SERIAL_JTAG
select MODULE_USB_BOARD_RESET if MODULE_STDIO_USB_SERIAL_JTAG
select MODULE_USBUS_CDC_ACM if MODULE_USBUS
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be more accurate to select this if the corresponding stdio is set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It cannot set the MODULE_STDIO_CDC_ACM without MODULE_USBUS_CDC_ACM which is why the MODULE_STDIO_USB_SERIAL_JTAG gets chosen.

Normally there would be a FORCE_USB_STDIO which would select the REQUIRES_USB_STDIO that would automatically bring it in with MODULE_USBUS,

Maybe instead we do:

Suggested change
select MODULE_USBUS_CDC_ACM if MODULE_USBUS
select REQUIRES_USB_STDIO if MODULE_USBUS || MODULE_TINYUSB_DEVICE

I just ran, it seems like it would pass:

$ ./dist/tools/compile_test/compile_like_murdock.py -j 8 -a examples/hello-world/ tests/sys/fido2_ctap/ tests/sys/usbus_cdc_acm_stdio/ tests/sys/usbus_hid/ tests/sys/usbus_board_reset/ tests/sys/usbus_cdc_ecm/ tests/sys/usbus_msc/ tests/pkg/tinyusb_cdc_acm_stdio/ tests/pkg/tinyusb_netdev/ tests/pkg/tinyusb_cdc_msc/  -b esp32s3-pros3 -v
examples/hello-world/          esp32s3-pros3                  PASS
tests/sys/fido2_ctap/          esp32s3-pros3                  PASS
tests/sys/usbus_cdc_acm_stdio/ esp32s3-pros3                  PASS
tests/sys/usbus_hid/           esp32s3-pros3                  PASS
tests/sys/usbus_board_reset/   esp32s3-pros3                  PASS
tests/sys/usbus_cdc_ecm/       esp32s3-pros3                  PASS
tests/sys/usbus_msc/           esp32s3-pros3                  PASS
tests/pkg/tinyusb_cdc_acm_stdio/ esp32s3-pros3                  PASS
tests/pkg/tinyusb_netdev/      esp32s3-pros3                  PASS
tests/pkg/tinyusb_cdc_msc/     esp32s3-pros3                  PASS
This took 0:07:24.991030...
You could have 17799.641200000002 meters driven in a Tesla

@MrKevinWeiss
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 6, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 95fb09c into RIOT-OS:master Jun 6, 2023
@MrKevinWeiss MrKevinWeiss deleted the pr/usbstuff branch June 7, 2023 06:55
@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: Kconfig Area: Kconfig integration CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants