[WIP] Class B fixes #481
[WIP] Class B fixes #481
Conversation
| @@ -711,7 +711,7 @@ static CONST_TABLE(u1_t, macCmdSize)[] = { | |||
| static u1_t getMacCmdSize(u1_t macCmd) { | |||
| if (macCmd < 2) | |||
| return 0; | |||
| if (macCmd >= LENOF_TABLE(macCmdSize) - 2) | |||
| if ((macCmd - 2) >= LENOF_TABLE(macCmdSize)) | |||
There was a problem hiding this comment.
Ugh. Sorry about this one.
There was a problem hiding this comment.
No problem, but I'm wondering why 0 is returned in the error case, because in scan_mac_cmds I see no check except for line 887, which doesn't do what the comment says.
There was a problem hiding this comment.
Yes, that's a bug too -- no test cases for bad mac commands, even in certification.
Should be if (cmdlen == 0 || cmdlen >> olen - oidx).
| @@ -1889,6 +1889,9 @@ static bit_t buildDataFrame (void) { | |||
| LMIC.frame[OFF_DAT_HDR] = HDR_FTYPE_DAUP | HDR_MAJOR_V1; | |||
| LMIC.frame[OFF_DAT_FCT] = (LMIC.dnConf | LMIC.adrEnabled | |||
| | (sendAdrAckReq() ? FCT_ADRACKReq : 0) | |||
| #if !defined(DISABLE_PING) | |||
| | ((LMIC.opmode & OP_PINGABLE) ? FCT_CLASSB : 0) | |||
There was a problem hiding this comment.
@terrillmoore I think LMIC.opmode & OP_PINGABLE is the wrong check, because this is true after LMIC_setPingable is called, but this doesn't mean that the periodic read has started (e.g. the device has no beacon received yet). Is OP_TRACK the right mask?
There was a problem hiding this comment.
I guess. The spec is not clear about this at all. Here's the relevant text (starting at line 1147, page 43, spec 1.0.3
The end-device application requests the LoRaWAN layer 1147 to switch to Class B mode. The LoRaWAN layer in the end-device searches for a beacon and returns either a BEACON_LOCKED service primitive to the application if a network beacon was found and locked or a BEACON_NOT_FOUND service primitive. To accelerate the beacon discovery the LoRaWAN layer MAY use the “DeviceTimeReq” MAC command.
Once in Class B mode, the MAC layer sets to 1 the Class B bit of the FCTRL field of every uplink frame transmitted.
From context, it does appear possible that they mean the device has to be locked to the beacon. So I think it makes sense to change to OP_TRACK, as that bit is set exactly when EV_BEACON_FOUND is reported.
There was a problem hiding this comment.
Fixing as part of #208 major update.
| @@ -236,7 +236,7 @@ ostime_t LMICeu868_nextTx(ostime_t now) { | |||
| #if !defined(DISABLE_BEACONS) | |||
| void LMICeu868_setBcnRxParams(void) { | |||
| LMIC.dataLen = 0; | |||
| LMIC.freq = LMIC.channelFreq[LMIC.bcnChnl] & ~(u4_t)3; | |||
| LMIC.freq = FREQ_BCN; | |||
There was a problem hiding this comment.
We need to implement MCMD_BeaconFreqReq at some point. When we do that, the FREQ_BCN can be overridden; so we need to get this from a data structure (eventually). We can introduce a new class B data structure to hold the beacon frequency and other eacon params.
BTW: we should remove CHNL_BCN altogether from lorabase.h; it should not be used to index LMIC.channels[], because those entries in LMIC.channels[] are used for other things and can't be safely used for the classB beacon.
Since we already have LMIC.ping.*, we could possibly add it there, or simply add LMIC.beacon.freq etc.
For the US, AU, and eventually CN470, we must deal with channel-roaming beacons; for that , we have to use LMIC.bcnChnl, but that is all done in the regional file. LMIC.bcnChnl can be moved into the "US-like" area of LMIC.
There was a problem hiding this comment.
Added first stage (static beacon frequency) for AS923, EU868, IN866 and KR920 as part of major changes for #208.
In this pull request I collect all fixes which I find, until class B is working (at least this is the goal).
Related to #479.