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

feat(cherryusb): add cherryusb device & host demo #603

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

Conversation

sakumisu
Copy link

#define FREERTOS_CONFIG_H

// This example uses a common include to avoid repetition
#include "FreeRTOSConfig_examples_common.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably just include https://github.com/raspberrypi/pico-examples/blob/develop/freertos/FreeRTOSConfig_examples_common.h rather than adding a separate copy of the file to the repo?

Copy link
Author

Choose a reason for hiding this comment

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

https://github.com/raspberrypi/pico-examples/tree/master/freertos i use this template, freertos uses FreeRTOSConfig.h not FreeRTOSConfig_examples_common.h, so i don't know why add FreeRTOSConfig_examples_common.h.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think maybe you could just do #include ../../../freertos/FreeRTOSConfig_examples_common.h in your FreeRTOSConfig.h? (although perhaps there's a "nicer" way of doing that without all the ../ ?) And then you could remove FreeRTOSConfig_examples_common.h from this PR. 🤷

Copy link
Author

@sakumisu sakumisu Jan 27, 2025

Choose a reason for hiding this comment

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

How about using FreeRTOSConfig.h only? If configs in FreeRTOSConfig_examples_common.h have conflict with other demo like less memory, no feature support like sem & mutex disabled, users will be confused.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm I just checked and I hadn't realised that FreeRTOSConfig_examples_common.h doesn't use the normal

#ifndef SOME_SETTING
#define SOME_SETTING some_value
#endif

pattern. But @peterharperuk understands all of this much better than I do! 😆

@@ -0,0 +1,295 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that this usb_config.h is identical to the other usb_config.h - perhaps it makes sense to add just one copy of the file to the repo (in usb/cherryusb/ ?), and have both examples share it? 🤔
(i.e. like what is done with https://github.com/raspberrypi/pico-examples/blob/develop/freertos/FreeRTOSConfig_examples_common.h and https://github.com/raspberrypi/pico-examples/blob/develop/pico_w/wifi/lwipopts_examples_common.h and https://github.com/raspberrypi/pico-examples/blob/develop/pico_w/wifi/mbedtls_config_examples_common.h )

Copy link
Author

Choose a reason for hiding this comment

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

This is an optional config for users to modify.

Copy link
Author

Choose a reason for hiding this comment

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

Like enable log, enable bufsize for performance, enable more class instances and so on.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still, perhaps it makes sense to have a per-example usb_config.h (where the users can add their optional customisation) which then does #include "usb_config_examples_common.h" ? (just to reduce the amount of duplication)

Copy link
Author

Choose a reason for hiding this comment

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

Still, perhaps it makes sense to have a per-example usb_config.h (where the users can add their optional customisation) which then does #include "usb_config_examples_common.h" ? (just to reduce the amount of duplication)

I know what you mean, yeah, reduce the duplication, also freertos. I will modify.

Copy link
Author

Choose a reason for hiding this comment

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

Have done. And i see pico_w demo has FreeRTOSConfig_examples_common.h, so i move this into config dir.

Copy link
Contributor

Choose a reason for hiding this comment

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

Have done.

Thanks 👍 But I'm still not sure if it makes sense to have both freertos/FreeRTOSConfig_examples_common.h and usb/cherryusb/config/FreeRTOSConfig_examples_common.h, which appear to be identical copies of each other?

@sakumisu
Copy link
Author

In addition, current directory structure i think need to modify.

@sakumisu sakumisu force-pushed the master branch 2 times, most recently from cf5327c to 4fd121d Compare January 27, 2025 14:29
Signed-off-by: sakumisu <1203593632@qq.com>
Signed-off-by: sakumisu <1203593632@qq.com>
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