Skip to content

Commit 19d90bd

Browse files
author
Kimmo Vaisanen
committed
Lora: Fix sticky MAC command retransmission
This commit fixes the bug where sticky MAC commands were duplicated in send buffer everytime send() was called.
1 parent 6c34802 commit 19d90bd

File tree

3 files changed

+20
-91
lines changed

3 files changed

+20
-91
lines changed

‎features/lorawan/lorastack/mac/LoRaMac.cpp

Lines changed: 20 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,7 @@ void LoRaMac::handle_data_frame(const uint8_t *const payload,
535535

536536
_params.adr_ack_counter = 0;
537537
_mac_commands.clear_repeat_buffer();
538+
_mac_commands.clear_command_buffer();
538539

539540
if (is_multicast) {
540541
_mcps_indication.type = MCPS_MULTICAST;
@@ -583,18 +584,9 @@ void LoRaMac::handle_data_frame(const uint8_t *const payload,
583584
_params.dl_frame_counter = downlink_counter;
584585
}
585586

586-
// This must be done before parsing the payload and the MAC commands.
587-
// We need to reset the MacCommandsBufferIndex here, since we need
588-
// to take retransmissions and repetitions into account. Error cases
589-
// will be handled in function OnMacStateCheckTimerEvent.
590-
if (_params.is_node_ack_requested) {
591-
if (fctrl.bits.ack) {
592-
_mac_commands.clear_command_buffer();
593-
_mcps_confirmation.ack_received = fctrl.bits.ack;
594-
_mcps_indication.is_ack_recvd = fctrl.bits.ack;
595-
}
596-
} else {
597-
_mac_commands.clear_command_buffer();
587+
if (_params.is_node_ack_requested && fctrl.bits.ack) {
588+
_mcps_confirmation.ack_received = fctrl.bits.ack;
589+
_mcps_indication.is_ack_recvd = fctrl.bits.ack;
598590
}
599591

600592
uint8_t frame_len = (size - 4) - app_payload_start_index;
@@ -663,6 +655,8 @@ void LoRaMac::on_radio_tx_done(lorawan_time_t timestamp)
663655
_lora_phy->set_last_tx_done(_params.channel, _is_nwk_joined, timestamp);
664656

665657
_params.timers.aggregated_last_tx_time = timestamp;
658+
659+
_mac_commands.clear_command_buffer();
666660
}
667661

668662
void LoRaMac::on_radio_rx_done(const uint8_t *const payload, uint16_t size,
@@ -781,7 +775,6 @@ bool LoRaMac::continue_joining_process()
781775
bool LoRaMac::continue_sending_process()
782776
{
783777
if (_params.ack_timeout_retry_counter > _params.max_ack_timeout_retries) {
784-
_mac_commands.clear_command_buffer();
785778
_lora_time.stop(_params.timers.ack_timeout_timer);
786779
return false;
787780
}
@@ -1193,7 +1186,6 @@ void LoRaMac::reset_mac_parameters(void)
11931186

11941187
_mac_commands.clear_command_buffer();
11951188
_mac_commands.clear_repeat_buffer();
1196-
_mac_commands.clear_mac_commands_in_next_tx();
11971189

11981190
_params.is_rx_window_enabled = true;
11991191

@@ -1598,26 +1590,23 @@ lorawan_status_t LoRaMac::prepare_frame(loramac_mhdr_t *machdr,
15981590
const uint8_t mac_commands_len = _mac_commands.get_mac_cmd_length();
15991591

16001592
if ((payload != NULL) && (_params.tx_buffer_len > 0)) {
1601-
if (_mac_commands.is_mac_command_in_next_tx() == true) {
1602-
if (mac_commands_len <= LORA_MAC_COMMAND_MAX_FOPTS_LENGTH) {
1603-
fctrl->bits.fopts_len += mac_commands_len;
1604-
1605-
// Update FCtrl field with new value of OptionsLength
1606-
_params.tx_buffer[0x05] = fctrl->value;
1607-
1608-
const uint8_t *buffer = _mac_commands.get_mac_commands_buffer();
1609-
for (i = 0; i < mac_commands_len; i++) {
1610-
_params.tx_buffer[pkt_header_len++] = buffer[i];
1611-
}
1612-
} else {
1613-
_params.tx_buffer_len = mac_commands_len;
1614-
payload = _mac_commands.get_mac_commands_buffer();
1615-
frame_port = 0;
1593+
if (mac_commands_len <= LORA_MAC_COMMAND_MAX_FOPTS_LENGTH) {
1594+
fctrl->bits.fopts_len += mac_commands_len;
1595+
1596+
// Update FCtrl field with new value of OptionsLength
1597+
_params.tx_buffer[0x05] = fctrl->value;
1598+
1599+
const uint8_t *buffer = _mac_commands.get_mac_commands_buffer();
1600+
for (i = 0; i < mac_commands_len; i++) {
1601+
_params.tx_buffer[pkt_header_len++] = buffer[i];
16161602
}
1603+
} else {
1604+
_params.tx_buffer_len = mac_commands_len;
1605+
payload = _mac_commands.get_mac_commands_buffer();
1606+
frame_port = 0;
16171607
}
16181608
} else {
1619-
if ((mac_commands_len > 0)
1620-
&& (_mac_commands.is_mac_command_in_next_tx() == true)) {
1609+
if (mac_commands_len > 0) {
16211610
_params.tx_buffer_len = mac_commands_len;
16221611
payload = _mac_commands.get_mac_commands_buffer();
16231612
frame_port = 0;
@@ -1634,7 +1623,6 @@ lorawan_status_t LoRaMac::prepare_frame(loramac_mhdr_t *machdr,
16341623
uint8_t *key = _params.keys.app_skey;
16351624
uint32_t key_length = sizeof(_params.keys.app_skey) * 8;
16361625
if (frame_port == 0) {
1637-
_mac_commands.clear_command_buffer();
16381626
key = _params.keys.nwk_skey;
16391627
key_length = sizeof(_params.keys.nwk_skey) * 8;
16401628
}
@@ -1811,7 +1799,6 @@ void LoRaMac::disconnect()
18111799

18121800
_mac_commands.clear_command_buffer();
18131801
_mac_commands.clear_repeat_buffer();
1814-
_mac_commands.clear_mac_commands_in_next_tx();
18151802

18161803
reset_mcps_confirmation();
18171804
reset_mlme_confirmation();

‎features/lorawan/lorastack/mac/LoRaMacCommand.cpp

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ static const uint8_t max_eirp_table[] = { 8, 10, 12, 13, 14, 16, 18, 20, 21, 24,
3636

3737
LoRaMacCommand::LoRaMacCommand()
3838
{
39-
mac_cmd_in_next_tx = false;
4039
sticky_mac_cmd = false;
4140
mac_cmd_buf_idx = 0;
4241
mac_cmd_buf_idx_to_repeat = 0;
@@ -99,15 +98,9 @@ void LoRaMacCommand::parse_mac_commands_to_repeat()
9998
}
10099
}
101100

102-
if (cmd_cnt > 0) {
103-
mac_cmd_in_next_tx = true;
104-
} else {
105-
mac_cmd_in_next_tx = false;
106-
}
107101
mac_cmd_buf_idx_to_repeat = cmd_cnt;
108102
}
109103

110-
111104
void LoRaMacCommand::clear_repeat_buffer()
112105
{
113106
mac_cmd_buf_idx_to_repeat = 0;
@@ -124,16 +117,6 @@ uint8_t LoRaMacCommand::get_repeat_commands_length() const
124117
return mac_cmd_buf_idx_to_repeat;
125118
}
126119

127-
void LoRaMacCommand::clear_mac_commands_in_next_tx()
128-
{
129-
mac_cmd_in_next_tx = false;
130-
}
131-
132-
bool LoRaMacCommand::is_mac_command_in_next_tx() const
133-
{
134-
return mac_cmd_in_next_tx;
135-
}
136-
137120
void LoRaMacCommand::clear_sticky_mac_cmd()
138121
{
139122
sticky_mac_cmd = false;
@@ -310,14 +293,6 @@ lorawan_status_t LoRaMacCommand::process_mac_commands(const uint8_t *payload, ui
310293
return ret_value;
311294
}
312295

313-
bool LoRaMacCommand::is_sticky_mac_command_pending()
314-
{
315-
if (mac_cmd_buf_idx_to_repeat > 0) {
316-
return true;
317-
}
318-
return false;
319-
}
320-
321296
int32_t LoRaMacCommand::cmd_buffer_remaining() const
322297
{
323298
// The maximum buffer length must take MAC commands to re-send into account.
@@ -336,7 +311,6 @@ lorawan_status_t LoRaMacCommand::add_link_check_req()
336311
mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_LINK_CHECK_REQ;
337312
// No payload for this command
338313
ret = LORAWAN_STATUS_OK;
339-
mac_cmd_in_next_tx = true;
340314
}
341315
return ret;
342316
}
@@ -348,7 +322,6 @@ lorawan_status_t LoRaMacCommand::add_link_adr_ans(uint8_t status)
348322
mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_LINK_ADR_ANS;
349323
mac_cmd_buffer[mac_cmd_buf_idx++] = status;
350324
ret = LORAWAN_STATUS_OK;
351-
mac_cmd_in_next_tx = true;
352325
}
353326
return ret;
354327
}
@@ -360,7 +333,6 @@ lorawan_status_t LoRaMacCommand::add_duty_cycle_ans()
360333
mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_DUTY_CYCLE_ANS;
361334
// No payload for this answer
362335
ret = LORAWAN_STATUS_OK;
363-
mac_cmd_in_next_tx = true;
364336
}
365337
return ret;
366338
}
@@ -375,7 +347,6 @@ lorawan_status_t LoRaMacCommand::add_rx_param_setup_ans(uint8_t status)
375347
// This is a sticky MAC command answer. Setup indication
376348
sticky_mac_cmd = true;
377349
ret = LORAWAN_STATUS_OK;
378-
mac_cmd_in_next_tx = true;
379350
}
380351
return ret;
381352
}
@@ -390,7 +361,6 @@ lorawan_status_t LoRaMacCommand::add_dev_status_ans(uint8_t battery, uint8_t mar
390361
mac_cmd_buffer[mac_cmd_buf_idx++] = battery;
391362
mac_cmd_buffer[mac_cmd_buf_idx++] = margin;
392363
ret = LORAWAN_STATUS_OK;
393-
mac_cmd_in_next_tx = true;
394364
}
395365
return ret;
396366
}
@@ -403,7 +373,6 @@ lorawan_status_t LoRaMacCommand::add_new_channel_ans(uint8_t status)
403373
// Status: Datarate range OK, Channel frequency OK
404374
mac_cmd_buffer[mac_cmd_buf_idx++] = status;
405375
ret = LORAWAN_STATUS_OK;
406-
mac_cmd_in_next_tx = true;
407376
}
408377
return ret;
409378
}
@@ -417,7 +386,6 @@ lorawan_status_t LoRaMacCommand::add_rx_timing_setup_ans()
417386
// This is a sticky MAC command answer. Setup indication
418387
sticky_mac_cmd = true;
419388
ret = LORAWAN_STATUS_OK;
420-
mac_cmd_in_next_tx = true;
421389
}
422390
return ret;
423391
}
@@ -429,7 +397,6 @@ lorawan_status_t LoRaMacCommand::add_tx_param_setup_ans()
429397
mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_TX_PARAM_SETUP_ANS;
430398
// No payload for this answer
431399
ret = LORAWAN_STATUS_OK;
432-
mac_cmd_in_next_tx = true;
433400
}
434401
return ret;
435402
}
@@ -444,7 +411,6 @@ lorawan_status_t LoRaMacCommand::add_dl_channel_ans(uint8_t status)
444411
// This is a sticky MAC command answer. Setup indication
445412
sticky_mac_cmd = true;
446413
ret = LORAWAN_STATUS_OK;
447-
mac_cmd_in_next_tx = true;
448414
}
449415
return ret;
450416
}

‎features/lorawan/lorastack/mac/LoRaMacCommand.h

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -97,18 +97,6 @@ class LoRaMacCommand {
9797
*/
9898
uint8_t get_repeat_commands_length() const;
9999

100-
/**
101-
* @brief Clear MAC commands in next TX.
102-
*/
103-
void clear_mac_commands_in_next_tx();
104-
105-
/**
106-
* @brief Check if MAC command buffer has commands to be sent in next TX
107-
*
108-
* @return status True: buffer has MAC commands to be sent, false: no commands in buffer
109-
*/
110-
bool is_mac_command_in_next_tx() const;
111-
112100
/**
113101
* @brief Clear sticky MAC commands.
114102
*/
@@ -132,13 +120,6 @@ class LoRaMacCommand {
132120
lora_mac_system_params_t& mac_params,
133121
LoRaPHY& lora_phy);
134122

135-
/**
136-
* @brief Verifies if sticky MAC commands are pending.
137-
*
138-
* @return [true: sticky MAC commands pending, false: No MAC commands pending]
139-
*/
140-
bool is_sticky_mac_command_pending();
141-
142123
/**
143124
* @brief Adds a new LinkCheckReq MAC command to be sent.
144125
*
@@ -237,11 +218,6 @@ class LoRaMacCommand {
237218
lorawan_status_t add_dl_channel_ans(uint8_t status);
238219

239220
private:
240-
/**
241-
* Indicates if the MAC layer wants to send MAC commands
242-
*/
243-
bool mac_cmd_in_next_tx;
244-
245221
/**
246222
* Indicates if there are any pending sticky MAC commands
247223
*/

0 commit comments

Comments
 (0)