PHDR_CRC

[sggoodri]

I implemented some code to check the validity of the phy header aka PHDR, using what Semtech calls the PHDR_CRC but is apparently really a checksum. My change is working on the signals I have demodulated so far.

It appears that the loraphy_header definition does not match the actual packed bit order of the crc_lsn and reserved (unused) nibbles; I swapped the order of the last two nibbles to work as follows:

typedef struct attribute((packed)) loraphy_header { uint8_t length; //first byte uint8_t crc_msn : 4; //lower four bits of second byte uint8_t has_mac_crc : 1; //fifth bit of second byte uint8_t cr : 3; //upper three bits of second byte uint8_t reserved : 4; //lower four bits of third byte uint8_t crc_lsn : 4; //upper four bits of third byte } loraphy_header_t;

So,

uint8_t received_checksum = ((d_phdr.crc_msn)&0x1)<<4 | d_phdr.crc_lsn;

can now be compared to

uint8_t calculated_checksum = headerChecksum(&d_decoded[0]);

using

inline uint8_t decoder_impl::headerChecksum(const uint8_t *h) { auto a0 = (h[0] >> 4) & 0x1; auto a1 = (h[0] >> 5) & 0x1; auto a2 = (h[0] >> 6) & 0x1; auto a3 = (h[0] >> 7) & 0x1;

        auto b0 = (h[0] >> 0) & 0x1;
        auto b1 = (h[0] >> 1) & 0x1;
        auto b2 = (h[0] >> 2) & 0x1;
        auto b3 = (h[0] >> 3) & 0x1;

        auto c0 = (h[1] >> 4) & 0x1;
        auto c1 = (h[1] >> 5) & 0x1;
        auto c2 = (h[1] >> 6) & 0x1;
        auto c3 = (h[1] >> 7) & 0x1;

        uint8_t res;
        res = (a0 ^ a1 ^ a2 ^ a3) << 4;
        res |= (a3 ^ b1 ^ b2 ^ b3 ^ c0) << 3;
        res |= (a2 ^ b0 ^ b3 ^ c1 ^ c3) << 2;
        res |= (a1 ^ b0 ^ b2 ^ c0 ^ c1 ^ c2) << 1;
        res |= a0 ^ b1 ^ c0 ^ c1 ^ c2 ^ c3;

        return res;
    }

The above checksum function is a modified version of the same named function at https://github.com/myriadrf/LoRa-SDR/blob/master/LoRaCodes.hpp

Does this approach work correctly for you, or have you already implemented this in other code not checked in on this project?

[rpp0]

Hi, thanks for taking the time to experiment with this. Since the header only contains 20 bits of data, determining how to store the last 4 bits in a struct is a matter of convention. Nevertheless, you are correct that the checksum is currently incorrectly stored in the reserved field. This is due to the following line: https://github.com/rpp0/gr-lora/blob/master/lib/decoder_impl.cc#L619. The zero nibble should actually be inserted at d_words_deshuffled.end()-1. But perhaps it is indeed better to simply swap the fields in the header definition like you suggested, since this corresponds to the left-to-right bit order of the PHY header when printed as raw bytes.

As for the checksum itself: I did implement a program to brute force which data bits are XORed in each bit of the checksum. The result can be found in https://github.com/rpp0/gr-lora/blob/master/include/lora/utilities.h#L388 and seems to correspond to the implementation of the LoRa-SDR project. It indeed works correctly, but I don't understand why, which is why I didn't implement it in gr-lora yet.

If you want, you can submit a PR for the new PHY header structure and put yourself in the list of contributors.

[sggoodri]

Thanks for the invite to be on the list of contributors, but I have moved on to another project now and just wanted to make sure that I shared my experience with the checksum so you can incorporate it as you wish. I was getting occasional bit errors in the header's length field that would create invalid length (often too long) packets that actually crashed the LoRaWAN dissector in the latest Wireshark. Filtering out the packets with bad header checksums before writing out to LoRaTAP/PCAP eliminated the problem.

[davidcastells]

Maybe I'm missing something. But I don't understand your comments about the position of bit fields...for instance

uint8_t crc_msn : 4; //lower four bits of second byte

the bit fields are defined linearly, so this should be "// upper four bits of second byte" , shouldn't it??