search

adafruit / Adafruit_CircuitPython_seesaw

seesaw helper IC driver for circuitPython
MIT License
60 stars 35 forks source link

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

Closed mkende closed 2 years ago

mkende commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Just for reference, that line was added in this commit: https://github.com/adafruit/Adafruit_CircuitPython_seesaw/commit/71f3947ff94ca070eb01c5f872b0400031be8086. But that commit is part of a PR about supporting INPUT_PULLDOWN.

ladyada commented 2 years ago

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

mkende commented 2 years ago

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 commented 2 years ago

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