Pull up required for reset pin?

[caternuson]

Is enabling internal pull up required for the reset pin? Or is simply setting to input adequate? Use case is for a board that lacks internal pull ups on inputs pins.

[jerryneedell]

In the arduino code -- it is just toggled https://github.com/adafruit/RadioHead/blob/master/examples/feather/Feather9x_TX/Feather9x_TX.ino#L90

Tony DiCola did the intial CP lib and did it that way -- I recall some discussion like "don't mess with it" ;-)

edited to add -- but looking at "blame" in the CP library -- it looks like I added the PULLUP. I think you can just toggle it as an output.

[jerryneedell]

@caternuson Do you want me to test it an put in a PR if it works?

[jerryneedell]

@caternuson reread you first post -- originally it was just setting it to input -- but for some reason, I added the PULLUPs.. I don't recall if there was a problem, but it seem like it does have to go high. SO why not leave it as an output and just toggle it? I think that is what Arduino does if I am reading it correctly.

[caternuson]

@jerryneedell I can test. I've swapped back to an Itsy to run some other tests. So can test with and without.

[jerryneedell]

OK -- but I don't trust just switching to input unless we have reason to believe that guarantees a PULLUP -- I'd feel safer just toggling it as an output.

[jerryneedell]

I suppose after the RESET is complete we could switch it back to input if that is preferred

[jerryneedell]

something like

def reset(self):
        """Perform a reset of the chip."""
        # See section 7.2.2 of the datasheet for reset description.
        self._reset.switch_to_output(value=False)
        time.sleep(0.0001)  # 100 us
        self._reset.value = True
        time.sleep(0.005)  # 5 ms
        self._reset.switch_to_input()

and this line has to be changed as well
https://github.com/adafruit/Adafruit_CircuitPython_RFM9x/blob/master/adafruit_rfm9x.py#L253

[jerryneedell]

@caternuson FYI #2

[caternuson]

Hmm. OK. So it seems like at some point it was needed. Could it be chip and/or CP version dependent?

I just ran this test and things seemed to work. I simply changed these two lines:

self._reset.switch_to_input(pull=digitalio.Pull.UP)

to this:

self._reset.switch_to_input()

so essentially undid that PR.

I've got another RFM9x setup on a Pi and had it send hello world. Then, on the Itsy M4, with the modified library, did this:

Adafruit CircuitPython 5.3.1 on 2020-07-13; Adafruit ItsyBitsy M4 Express with samd51g19
>>> import board
>>> import digitalio
>>> import adafruit_rfm9x
>>> cs = digitalio.DigitalInOut(board.D7)
>>> reset = digitalio.DigitalInOut(board.D9)
>>> rfm9x = adafruit_rfm9x.RFM9x(board.SPI(), cs, reset, 915.0)
>>> rfm9x.operation_mode
1
>>> packet = rfm9x.receive(timeout=5.0)
>>> packet
bytearray(b'hello world')
>>> 

[jerryneedell]

hmm -- makes me nervous -- I can play with it tomorrow and test it as an output -- unless you want to try that.

[caternuson]

I'd appreciate your second look at this. But this is low priority.

I will also continue to test things FWIW.

[jerryneedell]

@caternuson
I made the following changes and it seems to be woking fine.
This is similar to how the rfm69 i done (but the RFM69 is set High to RESET (normally low)

This is also how it is done in the Arduino RadioHead library.

I also tried setting it back to input after the reset and that also seems o be OK.
Does it make any difference? Is there any problem with leaving it as an output?

        # Setup reset as a digital output - initially High
        # This line is pulled low as an output quickly to trigger a reset.
        self._reset = reset
        # initialize Reset High
        self._reset.switch_to_output(value=True)
        self.reset()

...

    def reset(self):
        """Perform a reset of the chip."""
        # See section 7.2.2 of the datasheet for reset description.
        self._reset.value = False  # Set Reset Low
        time.sleep(0.0001)  # 100 us
        self._reset.value = True  # set Reset High
        time.sleep(0.005)  # 5 ms

here are the diffs to the repo

[Jerry-desktop-mini:~/projects/adafruit_github/Adafruit_CircuitPython_RFM9x] jerryneedell% git diff
diff --git a/adafruit_rfm9x.py b/adafruit_rfm9x.py
index 38b6180..1058249 100644
--- a/adafruit_rfm9x.py
+++ b/adafruit_rfm9x.py
@@ -31,7 +31,6 @@ http: www.airspayce.com/mikem/arduino/RadioHead/
 """
 import time
 import random
-import digitalio
 from micropython import const
 
 
@@ -245,12 +244,11 @@ class RFM9x:
         # Device support SPI mode 0 (polarity & phase = 0) up to a max of 10mhz.
         # Set Default Baudrate to 5MHz to avoid problems
         self._device = spidev.SPIDevice(spi, cs, baudrate=baudrate, polarity=0, phase=0)
-        # Setup reset as a digital input (default state for reset line according
-        # to the datasheet).  This line is pulled low as an output quickly to
-        # trigger a reset.  Note that reset MUST be done like this and set as
-        # a high impedence input or else the chip cannot change modes (trust me!).
+        # Setup reset as a digital output - initially High
+        # This line is pulled low as an output quickly to trigger a reset.
         self._reset = reset
-        self._reset.switch_to_input(pull=digitalio.Pull.UP)
+        # initialize Reset High
+        self._reset.switch_to_output(value=True)
         self.reset()
         # No device type check!  Catch an error from the very first request and
         # throw a nicer message to indicate possible wiring problems.
@@ -385,9 +383,9 @@ class RFM9x:
     def reset(self):
         """Perform a reset of the chip."""
         # See section 7.2.2 of the datasheet for reset description.
-        self._reset.switch_to_output(value=False)
+        self._reset.value = False  # Set Reset Low
         time.sleep(0.0001)  # 100 us
-        self._reset.switch_to_input(pull=digitalio.Pull.UP)
+        self._reset.value = True  # set Reset High
         time.sleep(0.005)  # 5 ms
 
     def idle(self):

[brentru]

FWIW - TinyLoRa performs a similar 100us toggle on the RST:
https://github.com/adafruit/Adafruit_CircuitPython_TinyLoRa/blob/master/adafruit_tinylora/adafruit_tinylora.py#L155

[jerryneedell]

@caternuson this seem to be working well for me -- I've tried it on a feather_m0_rfm9x, an stm32f405 with rfm9x feather wing and a Raspberry Pi 4 with Lora Bonnet. Would you like me to submit a PR?

[caternuson]

Thanks for all the testing. It seems to be working for me too. I'm still trying to test with FT232H with Blinka on Linux PC but running in to some other issues.

I'm a little concerned about all the code comment warnings. I don't want to undo some old black magic. This seemed to fix a previous issue (the PR you link above). Any idea what was happening there?

[jerryneedell]

I just vaguely recall that it broke when we moved from 2.0 to 3.0 and that seemed to fix it....
As you noted, it now seems to work with just being set to input, but I still think just toggling as an output is cleaner and consistent with all the other libraries TinyLora, RFM69 under CP and Radiohead under Arduino. The LMIC Lora driver (arduino) does some more fiddling with it.....

If you want a reliable way without internal Pull-ups, I am more comfortable with this method then just removing the pull-up settings.

I'll be happy to defer to anyone with a good justification for going back to the old way that failed for 3.0 but seems to work now.

The only issue I see with setting the output to High is the note that the RESET pin should be floating at POR but I don't know of any case where we would be doing that since it should be connected at power on.

[caternuson]

It's also a little odd that the datasheet explicitly labels the diagram above with "High-Z" and seems to indicate the high/low state doesn't matter. Why not just show a basic HIGH - LOW - HIGH pattern? Maybe the info helps when designing a hardware reset, like with a button?

I fell down a rabbit hole testing with FT232H. Found this at the bottom:
adafruit/Adafruit_Blinka#326
Let's hold off until that gets resolved and make sure things work with FT232H. That's an odd ball enough use case that it will help shake things out a little better.

[jerryneedell]

No problem -- I'm still puzzled by the data-sheet -- Not clear to me which side the diagram is talking about. the RFM9x side or the MCU side....

[caternuson]

@jerryneedell OK, issues with FT232H resolved and I'm now testing with the change-reset-to-digital-out update. It seems to work fine.

For TX, I'm using a RPi with LoRa bonnet:

(blinka) pi@raspberrypi:~/lora $ python3
Python 3.7.3 (default, Jul 25 2020, 13:03:44) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import board
>>> import digitalio
>>> import adafruit_rfm9x
>>> CS = digitalio.DigitalInOut(board.CE1)
>>> RESET = digitalio.DigitalInOut(board.D25)
>>> rfm9x = adafruit_rfm9x.RFM9x(board.SPI(), CS, RESET, 915.0)
>>> rfm9x.send(b'hello world')
True
>>> 

And for RX, using a LoRa breakout with an FT232H connected to a linux box.

(blinka) linux:~/lora$ python3
Python 3.6.9 (default, Jul 17 2020, 12:50:27) 
[GCC 8.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import board
>>> import digitalio
>>> import adafruit_rfm9x
>>> cs = digitalio.DigitalInOut(board.C0)
>>> reset = digitalio.DigitalInOut(board.C1)
>>> rfm9x = adafruit_rfm9x.RFM9x(board.SPI(), cs, reset, 915.0)
>>> rfm9x.receive(timeout=5)
bytearray(b'hello world')
>>>

Since it seems to work and the same approach is being taken by other libraries, I guess this change is OK. And if this turns out to be wrong, then we have all this discussion to document things, and can simply revert.

Do you want to make a PR?

[jerryneedell]

Sure . All set to submit.

[jerryneedell]

resolved by #47