search

mcci-catena / arduino-lmic

LoraWAN-MAC-in-C library, adapted to run under the Arduino environment
https://forum.mcci.io/c/device-software/arduino-lmic/
MIT License
636 stars 207 forks source link

Add support for RegInvertIQ2 #787

Open zhgzhg opened 2 years ago

zhgzhg commented 2 years ago

Greetings, The datasheets of the SX127x series and the corresponding application notes suggest modifying RegInvertIQ2 when receiving LORA downlinks. I couldn't find any traces of that insinde src/lmic/radio.c, thus I wanted to address it.

The docs:

SX1276-7-8-9 Datasheet, the end of page 115 SX1272 Datasheet, the end of page 113 AN 1200.24, the end of page 6

Environment

cstratton commented 2 years ago

Not sure what you think needs to be done "to address it" - the code which configures the needed IQ inversion mode is right here:

https://github.com/mcci-catena/arduino-lmic/blob/master/src/lmic/radio.c#L941

And yes, it works - set it wrong and you'll find you only occasionally receive transmitted packets in circumstances where things are so good that the signal can be decoded even though the decoder is looking for its conjugate

zhgzhg commented 2 years ago

@cstratton , at https://github.com/mcci-catena/arduino-lmic/blob/master/src/lmic/radio.c#L941 I can see only RegInvertIQ as being modified. The documentation from the links I've given above suggest modifying RegInvertIQ2 as well. I'm not sure if the implementation deliberately is not working with RegInvertIQ2, or it has been missed.

cstratton commented 2 years ago

Ah, ok, sorry missed the IQ2 part. It's kind of odd that they've only revealed the details of these chips bit by bit, doesn't seem like this part of the radio code has been touched since before the announcement of that really propogated.

I see that the LoRaMac-node code does set this, probably worthwhile to adopt that

https://github.com/Lora-net/LoRaMac-node/blob/master/src/radio/sx1276/sx1276.c#L842

Have you tried adding this register setting to your own local copy of LMiC?

terrillmoore commented 2 years ago

@zhgzhg the datasheet I have does not mention this register or these bits. Thanks for bringing this to our attention, but @cstratton is right; this code was all implemented long before that datasheet change was announced.

zhgzhg commented 2 years ago

Got it, thanks @terrillmoore .

Yes, I've tried modifying the register in my local LMiC copy, @cstratton . I couldn't notice any negative effects, but at the moment I don't have proper test environment to say confidently.

terrillmoore commented 2 years ago

Curiously, that's in the transmit path. Our use of the noIqInversion is in the RX path. The sample code doesn't seem to use noIqInversion during receive! (And the default is, I think, in the proper state.)