chirpstack / chirpstack

ChirpStack open-source LoRaWAN Network Server
https://www.chirpstack.io/
MIT License
572 stars 152 forks source link

Processing of mac commands seem unstable #424

Closed pnxs closed 5 months ago

pnxs commented 5 months ago

I observed a problem with DevStatusAns, my battery values where always wrong. I investigated and found out, that dev_status.rs:handle() just checks, that the first MACCommand in 'block' is valid, regardless of the variant. The later code does a "if let lrwn::MACCommand::DevStatusAns(pl) = mac" but when the first MACCommand isn't a DevStatusAns, it goes silently unprocessed. I assume a problem with the implementation in src/maccomand/mod.rs:handle_uplink() "Iterate over mac-commands in order of CID". My understanding of a) RUST and b) chirpstack is not enough to decide how to solve the problem. I experimentally extended the testcase in dev_status.rs by just inserting another maccommand in the vector

        let block = lrwn::MACCommandSet::new(vec![
            lrwn::MACCommand::LinkADRAns(
                lrwn::LinkADRAnsPayload {
                    ch_mask_ack: true,
                    dr_ack: true,
                    tx_power_ack: true,
                }
            ),
            lrwn::MACCommand::DevStatusAns(
                lrwn::DevStatusAnsPayload {
                    battery: 254,
                    margin: 10,
                },
            )
        ]);

The test failes after that, because the DevStatusAns is not processed anymore after that. I then add a search for DevStatusAns in dev_status.rs::handle() which searches for that variant in 'block'. The test works out fine with that change.

Isn't a checking of the expected type in the maccommand handlers generally a good idea? What is the use of iterating over the CIDs in order?

brocaar commented 5 months ago

Your test test a scenario that will never happens. ChirpStack first collects mac-commands with equal CID in 'blocks', thus all mac-commands within a 'block' share the same CID. See also https://github.com/chirpstack/chirpstack/blob/master/chirpstack/src/maccommand/mod.rs#L52.

If you think there is an issue with reporting the battery value, please post the raw mac-command payload + the value that ChirpStack reports.

pnxs commented 5 months ago

I'm very certain, that there is a problem (all of my devices are reported to have 3% battery). I became aware of, because I looked at the "f_opts" array. This contained to mac-commands: 1. LinkADRAns and 2. DevStatusAns. I know from my device, that the first payload got the value of 7, the DevStatusAns e.g. 200. I think, the payload of LinkADRAns is processed with the Interpretation of DevStatusAns. 7 => 3% battery value.

I glad to retrieve some raw data, but can you give me a hint, how to get the raw mac-command payload?

brocaar commented 5 months ago

The easiest way would be to look at the LoRaWAN frames of the device in the web-interface.

On Mon, May 20, 2024, 09:16 Thomas Schätzlein @.***> wrote:

I'm very certain, that there is a problem (all of my devices are reported to have 3% battery). I became aware of, because I looked at the "f_opts" array. This contained to mac-commands: 1. LinkADRAns and 2. DevStatusAns. I know from my device, that the first payload got the value of 7, the DevStatusAns e.g. 200. I think, the payload of LinkADRAns is processed with the Interpretation of DevStatusAns. 7 => 3% battery value.

I glad to retrieve some raw data, but can you give me a hint, how to get the raw mac-command payload?

— Reply to this email directly, view it on GitHub https://github.com/chirpstack/chirpstack/issues/424#issuecomment-2119919270, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIM6LLCKIHWB6C6CNTYDTZDGWN7AVCNFSM6AAAAABH6ISMP6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZHEYTSMRXGA . You are receiving this because you commented.Message ID: @.***>

pnxs commented 5 months ago

The mac-commands are not included raw in the lorawan-frame view... but I found the problem and it is not chirpstack :-) Due to a unlucky coincidence of values (the 7) and the missing raw data, I was mislead. My Device just got the DevStatusAns payload wrong (battery and margin where interchanged) and my devices reports 7 as margin... nevermind. Btw: This is a Bug in RadioLib... I'll bring that up there.