adafruit / Adafruit_CircuitPython_seesaw

seesaw helper IC driver for circuitPython
MIT License
62 stars 36 forks source link

seesaw digitalio not setting pullups properly #84

Open kattni opened 3 years ago

kattni commented 3 years ago

The following code should work.

import time
import board
from adafruit_seesaw import seesaw, digitalio

ss = seesaw.Seesaw(board.I2C())

button = digitalio.DigitalIO(ss, 0)
button.switch_to_input(pull=ss.INPUT_PULLUP)

while True:
    print(button.value)
    time.sleep(0.2)

However, it does not. The pin is left floating. The following code, does work.

import time
import board
import digitalio
from adafruit_seesaw.seesaw import Seesaw
from adafruit_seesaw.digitalio import DigitalIO

ss = Seesaw(board.I2C())

button = DigitalIO(ss, 0)
button.direction = digitalio.Direction.INPUT
button.pull = digitalio.Pull.UP

while True:
    print(button.value)
    time.sleep(0.2)

Presumably the first example is supposed to work like the second example, but it does not. I have no suggestions on where to start with fixing this.

caternuson commented 3 years ago

The checks being done in adafruit_seesaw.digitalio.py are based on the core digitalio library defines. But the Seesaw class in adafruit_seesaw.seesaw.py also defines its own set. Mixing these (first example above) is what is leading to this issue.

adafruit_seesaw.Seesaw.INPUT_PULLUP != digitalio.Direction.INPUT_PULLUP

Not sure what the proper solution is. Open for discussion. :)

jepler commented 3 years ago

Should the incorrect code be detected as an error, like

diff --git a/adafruit_seesaw/digitalio.py b/adafruit_seesaw/digitalio.py
index ca9a1f4..4d84be7 100644
--- a/adafruit_seesaw/digitalio.py
+++ b/adafruit_seesaw/digitalio.py
@@ -49,8 +49,10 @@ class DigitalIO:
             self._seesaw.pin_mode(self._pin, self._seesaw.INPUT_PULLDOWN)
         elif pull == digitalio.Pull.UP:
             self._seesaw.pin_mode(self._pin, self._seesaw.INPUT_PULLUP)
-        else:
+        elif pull is None:
             self._seesaw.pin_mode(self._pin, self._seesaw.INPUT)
+        else:
+            raise ValueError("Out of range")
         self._pull = pull

     @property
caternuson commented 3 years ago

That'd help with visibility, since it would gripe instead of silently accepting and doing something unexpected.

I guess this could be considered a bug in user code:

button.switch_to_input(pull=ss.INPUT_PULLUP)

since ss.INPUT_PULLUP is a mode, not a pull.