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 make Serial and Serial1 def and ISR optional #632

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maxlem
Copy link

@maxlem maxlem commented May 21, 2021

see also this discussion:
#631

@CLAassistant
Copy link

CLAassistant commented May 21, 2021

CLA assistant check
All committers have signed the CLA.

@maxlem
Copy link
Author

maxlem commented May 21, 2021

I'm willing to port the defines to the other samd variant.cpp

@matthijskooijman
Copy link
Collaborator

What's your idea about setting these defines? You can't really set defines from a sketch, so are you thinking about a custom boards.txt that reuses the variant but sets these defines in the extra_flags or something like that?

Looking at the code, I think this now prevents definition of e.g. Serial or Serial1, but not the declaration, leading to linker error. I would think it would make sense to also not declare the objects at all, so you would get a compiler error earlier if you tried to use them, but I'm not quite sure where they are declared exactly.

@matthijskooijman
Copy link
Collaborator

(Also, I'm just offering some thoughts on the PR since I participated in the issue, I'm not at all sure if adding these guards is the best approach, but I'm also not in a position to decide on whether to merge this or not).

@maxlem
Copy link
Author

maxlem commented May 22, 2021

Yes we might just as well not neclare them, I was just shooting for the most minimal changes.

The use case would me to add -D ARDUINO_ZERO_CUSTOM_SERCOM0_SERIAL to the build command.

I don't know how to do that in Arduino IDE, I use platformio

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