matthias-bs / loraE22

A MicroPython class for the Ebyte E22 Series LoRa modules
GNU General Public License v3.0
10 stars 3 forks source link

recvMessage signature and comment misleading? #2

Open Bikeman opened 2 years ago

Bikeman commented 2 years ago

I'm a bit puzzled by the signature and logic behind the recvMessage method:

It has mandatory arguments from_address and from_channel, but those are just compared to the configured address and channel and depending on the result, the E22 is either switched to fixed or transparent mode (if mode changed). However, if your E22 was set to channel X,in the current code, it doesn't work to use a channel other than X in the recvMessage method anyway because the method does NOT try to change the channel, even if it is different from the configured value. So calling e22.recvMessage(some_address,X+1) will actually still listen for messages on the initially configured channel X, not X+1.

So I wonder what was the intended use case in the first place. Is it a bug that the channel switching was left out?

But the from_address parameter is problematic as well: Contrary to the comment for this method, the from_address cannot actually restrict (by numerical address) who can sent messages to the E22 (other than enabling transparent mode and implicitly restricting sender_address==target_address, unless message was sent as broadcast). In fixed mode, it will receive messages from all stations on the same channel that sent fixed mode messages with the target address matching the address of the receiving E22. As I understand the specs of the E22, it is not possible to filter on, or even extract afterwards, the address of the sending station.

So in summary I think the signature and comment of this method is misleading and the the described functionality is just not possible to achieve. So one should probably change the signature altogether.

Would it make more sense perhaps to have a recvMessage(self) method that just checks for available messages in the buffer of the E22 and a separate method setChannel(self,channel) (there is already a "public" method to set the transmission mode).

In that case the signature of the sendMessage method could stay the same, perhaps changing the logic a bit to just ignore channel and address when called in transparent mode, so no longer trying to switch between modes based on comparison of target address and channel with configured values. to_address and to_channel could then also be optional (not needed in transparent mode).

matthias-bs commented 2 years ago

matthias-bs/loraE22 is basically a port of effevee/loraE32 from the E32 to the E22 device.

So chances are that

  1. I took over something that was not quite correct in the loraE32 code
  2. That I made an error while porting
  3. That I did not take care of a possible difference between E32 and E22

I must admit that I did not really make myself familiar with the different transmission modes. Neither did I make a proper validation of each mode.

I stopped working on it after the communication basically worked and I was able to do some transmission range tests. My initial goal was to build a LoRaWAN/TTN node. After I found out that the E22 module I bought was not able to provide the required functionality, my goal was still to do 'something useful' with it.

Long story short: You are probably right with your findings. I would really appreciate any fixes you are going to make.