Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
since in_isr() is HW specific anyway, why not just include required file here explicitly?
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.
I suppose all ARM MCUs have the SCB so it should be enough to include a header file for the HW specific stuff. At least in my case (stm32) it is. I suppose everyone who use this code needs to add an include statement to this file (or modify it further).
It is a bit cumbersome to maintain changes to the nimble repo at our end since we want to use the nimble repo as a submodule and it is very close to work without changes. Having a possibility to specify a file to include here makes it possible for us to use nimble without any changes.
Perhaps there is a CMSIS file that can be included that specifies the SCB? That might introduce some unwanted dependencies though...
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.
fair enough, but could you put some comment above this include? explaining why it is needed etc
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.
Certainly. I have added a comment now and I tried to keep it short but descriptive. To me, it is borderline obvious that you use this to avoid changes in the code and if other changes are required anyway then I guess people simply won't bother with this. Let me know what you think and I'll update.