-
Notifications
You must be signed in to change notification settings - Fork 36
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
Include refactoring #121
Include refactoring #121
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
366ab0f
to
2403681
Compare
This comment was marked as outdated.
This comment was marked as outdated.
2403681
to
6283958
Compare
This comment was marked as outdated.
This comment was marked as outdated.
6283958
to
d8586b2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
d8586b2
to
6569c17
Compare
This comment was marked as outdated.
This comment was marked as outdated.
e8b1fb7
to
9f19e5b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
5853fe0
to
f39958c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andreagilardoni I've got two proposals to improve this PR:
- Add
#ifdef BOARD_HAS_XXX
also in the various Connectionhandler .h files. This would allow us to add something like
#ifdef BOARD_HAS_XXX
...
#else
#error Board do not support NBConnectionHandler
#endif
and this should trigger a nice error if you build for example a sketch for Portenta H7 including Arduino_NBConnectionHandler.h
- Remove
Arduino_
from all ConnectionHandlers implementation *.h *.cpp files to make more clear that the official entrypoint of the library is onlyArduino_ConnectionHandler.h
This would not prevent the users to include directlyNBConnectionHandler.h
, but would be a strong suggestion and we can leave in place also the#ifdef BOARD_HAS_XXX
check
Removing Arduino_
from the implementation files will also free the names so we can reuse them to make some ifdef magic and configure the library from an external library.
This comment was marked as resolved.
This comment was marked as resolved.
1cbd977
to
62ad2b8
Compare
Memory usage change @ bc9691b
Click for full report table
Click for full report CSV
|
This PR aims to split the entrypoint for this library into definitions of macro config values, connectionHandlerInterface. This allows to avoid cyclic includes in the extension of functionalities of this library.