Skip to content

Do not blank out the value read for pins PA30 and PA31. #98

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

Merged
merged 1 commit into from
Apr 5, 2022
Merged

Do not blank out the value read for pins PA30 and PA31. #98

merged 1 commit into from
Apr 5, 2022

Conversation

mkende
Copy link
Contributor

@mkende mkende commented Apr 4, 2022

Pins PA30 and PA31 are the SWDIO and SWDCLK pins. But nothing prevents them from being used as GPIO when a debugger is not connected.

So this PR stops masking the two bits corresponding to these pins so that they can be read if needed. As the user supplied pins mask is still applied, this should have no impact on code that was not already trying to read these pins.

@ladyada
Copy link
Member

ladyada commented Apr 5, 2022

it depends what board is on the other side! this should be fixed in the firmware on the breakout not the library which does not know what pins are which

@mkende
Copy link
Contributor Author

mkende commented Apr 5, 2022

I agree that this sort of things should be fixed in the firmware but, precisely, here I’m unfixing the Circuit Python code: that code is currently erasing the last two bits of data sent by the chip while there is no reason to do that in general. And, yes, If a particular chip can’t send meaningful data in these bits, they should be erased in the firmware used by that chip rather than here in Python.

For some context, the board that I’m using is an ATSAMD10 in a SOIC-14 form factor, so I have only a few precious pins and need to use the pins 30 and 31. But I believe that this would actually work too also on the the ATSAMD09 breakout if not for this line that I’m removing in the python code.

@dhalbert
Copy link
Contributor

dhalbert commented Apr 5, 2022

Just for reference, that line was added in this commit: 71f3947. But that commit is part of a PR about supporting INPUT_PULLDOWN.

@ladyada
Copy link
Member

ladyada commented Apr 5, 2022

right! ok well i guess lets merge this and see if anthing breaks? :)

@ladyada ladyada merged commit 3aaf721 into adafruit:main Apr 5, 2022
@mkende
Copy link
Contributor Author

mkende commented Apr 5, 2022

I can see why that other commit did that, as pin 30 (but not 31) needs to be externally pulled-up, so it would not work as a pull-down input (for these ATSAMD chips at least). The earlier point stands that it was probably not the right place to address this sort of things.

Anyway, thanks a lot for the fast review. I appreciate this very much (and the fact that @ladyada personally handles these).

@ladyada
Copy link
Member

ladyada commented Apr 5, 2022

thanks, if you find more seesaw bugs just open PRs and eventually ill understand what they mean wheee

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Apr 8, 2022
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