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

NFC to GPIO target.json comprehension issue #465

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Joseph-Tasker
Copy link

The microbit library contains a file called target.json which is used to create definitions which interact with the nRF52833 SDK. One of these is "CONFIG_GPIO_AS_PINRESET" which changes the functionality of pin 8 and 9 from NFC antennas to GPIO pins 1 and 2.
However, the SDK checks if the definition has been defined and not the value it holds. Due to this, NFC is not possible if the value is set to 0 as interpreted, rather the solution is to delete/comment the line so the definition is never defined. This information has been added to the README, but this may not be the intended format of the maintainer.

Updated the target.json and README to indicate the purpose and functionality of the property due to the previous confusion of a true or false value when its a if defined option.
Corrected the subheading to be the correct size.
@finneyj
Copy link
Contributor

finneyj commented Feb 18, 2025

Thanks @Joseph-Tasker.

I think you've happened upon a more general issue here, of how we should handle defining CONFIG options for compiler flags that test the presence of a macro rather than its value.

I quite like the textual "save the fellow person" approach, but the text string is quite long, and is specific to this particular issue, and couldn't be reused.

I wonder if we could have a shorter message, and something more general. e.g. "Enabled. Delete to disable."

or similar??

Changed text to used repeatedly on other variables facing the same issue.
@Joseph-Tasker
Copy link
Author

Thanks for the feedback. I've made a change to reflect the feedback where the string now states:

"ENABLED - Delete entire line to Disable"

I believe the use of the work 'entire' makes it clear that the whole line needs to be removed. After asking a few other people they also agreed as they said they world remove the string rather than the whole line. This should also be useable so that other CONFIG options with the same issue can use this line.

Regarding the issue, would it be preferable if I were to make a new request fixing them or to continue this branch and find any other variables with this issue. The one problem is that I can only find these issues when the libraries have been built which I can do in my other projects.

I hope this is the solution you are looking for.

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