local variable reference before assignment in rfm69.py #5

ndgipper · 2024-12-04T02:53:54Z

read_fifo in rfm69.py tries to return packet when it is possibly not assigned

jerryneedell · 2024-12-04T09:25:52Z

Can you provide more information? Can you post the code that you are running? What error messages do you receive? The full traceback would be helpful.

Looking at the code, it does look like it should be changed to return None. I'll do some testing.

I'm surprised I have not seen this error so it would be helpful to see what code you are running so I can try to reproduce it.

edited to add: It is clear that read_fifo "can" try to return packet without it being initialized if it is called when the FIFO is empty, but the code "should" never call read_fifo if the fifo is empty. I'd like to understand haw that is happening in your case. Are you calling read_fifo directly from your code or using the receive function?

jerryneedell · 2024-12-04T17:27:47Z

Although I cannot reproduce your issue, I think it does make sense to make sure "packet" is always defined.
I added a line to set packet=None before reading the FIFO.

    def read_fifo(self) -> bytearray:
        """Read the packet from the FIFO."""
        # Read the length of the FIFO.
        fifo_length = self.read_u8(_RF69_REG_00_FIFO)
        packet = None        # Return None if FIFO empty
        if fifo_length > 0:  # read and clear the FIFO if anything in it
            packet = bytearray(fifo_length)
            # read the packet
            self.read_into(_RF69_REG_00_FIFO, packet, fifo_length)
        return packet

I'll do a bit more testing and then submit a PR to implement this change.

As I noted, it is not clear to me how read_fifo can be getting called when the FIFO is empty. read_fifo should only be called if there is something in the FIFO.

It would be good if you can try adding the packet = None line as above to the code you are running and let me know if it eliminates the problem.

ndgipper · 2024-12-05T00:03:10Z

Here is the Traceback before making any changes to software:


Received bytes: bytearray(b'\x01\x02@\x02\x00\x12\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00RELAY V1.0\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x0)
147584
Traceback (most recent call last):
  File "code.py", line 59, in <module>
  File "adafruit_rfm/rfm_common.py", line 72, in blocking_function
  File "asyncio/core.py", line 317, in run
  File "asyncio/core.py", line 276, in run_until_complete
  File "asyncio/core.py", line 261, in run_until_complete
  File "adafruit_rfm/rfm_common.py", line 432, in asyncio_receive
  File "adafruit_rfm/rfm69.py", line 650, in read_fifo
NameError: local variable referenced before assignment

Code done running.

Press any key to enter the REPL. Use CTRL-D to reload.

Those received bytes are all correct. The dataframe is as it should be.

That integer printed just before the traceback is a gc.free_mem() value. I was looking at the memory situation. All ok there.

At the present, I've added the packet = None line to the rfm69.py file and restarted the code. I'll let it run a while and see what happens.

Some speculation.
My miniWireless remote node (lowpowerlabs c code) sends 3 messages that each have a retry (limit of 5) that fire together about once every 30 seconds. Since the remote receives no ACK, there are 5 copies of each message. So the burst is total of 15 messages. Each of a length of 49 bytes. It looks like the python keeps up with it. But, not always! just as I was typing this it received a garbled packet of only 15 bytes. My eventual python code would reject such a packet. I've yet to think of a reason why the FIFO would indicate a packet is ready but not have anything. My remote node is only sending the same 3 messages over and over..

I suspect the packet = None will fix the crash. But, I'm not yet sure that we have an assurance that the python can keep up with the burst I'm sending. I'm still accepting that my long term solution might be to replace the remote nodes with Circuitpython nodes. But, I'm still curious about this project we're on.

I'll post more if something comes to mind.
I'm going to start to think more about the ACK handshake in software - which is how my original LowPowerLabs setup on both sides worked - ACK handshake was all in the software (albeit fast C code)

Meanwhile, I'm going to leave this setup running for several hours.

Here's my "gateway" node code that is just listening to the messages from one remote node.
Please note the console output just above the traceback above does not go with this code. I've changed the print since the output above was generated.

import time
import board
import busio
import digitalio
from adafruit_rfm import rfm69
import gc

RADIO_FREQ_MHZ = 915.0  # Frequency of the radio in Mhz. Must match your

# Define pins connected to the chip, use these if wiring up the breakout according to the guide:
CS = digitalio.DigitalInOut(board.D5)
RESET = digitalio.DigitalInOut(board.D6)
# set the time interval (seconds) for sending packets
transmit_interval = 5
# set destination address
destination_node = 2
# set this node address
my_node = 1
# set newtwork ID -- used for sync word
network_id = 1

# Initialize SPI bus.
spi = busio.SPI(board.SCK, MOSI=board.MOSI, MISO=board.MISO)
#crate sync_word to match Low Power Labs convention 
sync = bytearray(2)
sync[0] = 0x2d
sync[1] = network_id
rfm69 = rfm69.RFM69(spi, CS, RESET, RADIO_FREQ_MHZ,preamble_length=3,sync_word=sync,crc=True)
# set the time interval (seconds) for sending packets
rfm69.radiohead = False
# Optionally set an encryption key (16 byte AES key). MUST match both
# on the transmitter and receiver (or be set to None to disable/the default).
#rfm69.encryption_key = None
rfm69.encryption_key = (
   b"1234567891011121"
)
# configure for compatibility with Low Power Lab library defaults
rfm69.modulation_shaping = 0
rfm69.bitrate = 55555
rfm69.frequency_deviation = 50000
rfm69.dc_free = 0
# Wait to receive packets.
print("Waiting for packets...")
# initialize flag and timer
time_now = time.monotonic()
# create payload 
payload = bytearray(5)
# first three bytes are the  header
payload[0] = destination_node
payload[1] = my_node
# third header byte is for ACK :  0 = no ACK
payload[2] = 0x0
# two more bytes for a counter
payload[3] = 0x0
payload[4] = 0x0
counter = 0
while True:
    # Look for a new packet - wait up to 2 seconds:
    packet = rfm69.receive(timeout=2.0)
    # If no packet was received during the timeout then None is returned.
    if packet is not None:
        # Received a packet!
        # Print out the raw bytes of the packet:
        #print("Received bytes: {0}".format(packet))
        print(gc.mem_free())
        print(len(packet),' :',packet.hex())
        # send reading after any packet received
    #if time.monotonic() - time_now > transmit_interval:
    #    counter = (counter + 1) & 0xffff
    #    payload[3] = counter >> 8
    #    payload[4] = counter & 0xff
    #    rfm69.send(payload)
    #    time_now = time.monotonic()

ndgipper · 2024-12-05T00:39:10Z

FYI
I've just added a software ACK response (snippets below) and the remote node now accepts the ACK and only sends one message for each of the 3 (no retries). Success!

So, far there are a few extraneous packets every so often. My software ACK works. And no code crash after adding the packet=None line to the library file.

I'll post more if I learn more

snippets:

ackpayload = bytearray(3)
ackpayload[0] = 2     # who is receiving the ACK  node 2 is the remote node
ackpayload[1] = 1     # who is sending the ACK
ackpayload[2] = 0x80    # the ACK value for the CTLbyte

rfm69.send(ackpayload)

jerryneedell · 2024-12-05T13:23:24Z

It sounds like you are making great progress in getting your project working. I'm happy to see someone is actually able to take advantage of some of the new features of the RFM library (disabling the RadioHead header in your case). It is nice to see the ACK working. As I noted, I expect it will be OK for the received packets. It may be harder to implement for transmitted packets. You will still likely see some packet loss/corruption but hopefully a tolerable amount. Good luck with it.

I will go ahead an submit a PR later today to add the packet = None to the read_fifo functions. (It is the same for rfm9x and rfm9xfsk) It is still a mystery why you are receiving empty packets, but at least now they will be legitimately ignored.

jerryneedell self-assigned this Dec 4, 2024

jerryneedell added the bug Something isn't working label Dec 4, 2024

jerryneedell mentioned this issue Dec 5, 2024

fix read_fifo to always define packet before return, Correct typo in dosctring #6

Merged

FoamyGuy closed this as completed in #6 Dec 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

local variable reference before assignment in rfm69.py #5

local variable reference before assignment in rfm69.py #5

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

local variable reference before assignment in rfm69.py #5

local variable reference before assignment in rfm69.py #5

Comments

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!