Skip to content

Commit ca0aa76

Browse files
Kimmo Vaisanen0xc0170
Kimmo Vaisanen
authored and
0xc0170
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 ca920c6 commit ca0aa76

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
}
@@ -1195,7 +1188,6 @@ void LoRaMac::reset_mac_parameters(void)
11951188

11961189
_mac_commands.clear_command_buffer();
11971190
_mac_commands.clear_repeat_buffer();
1198-
_mac_commands.clear_mac_commands_in_next_tx();
11991191

12001192
_params.is_rx_window_enabled = true;
12011193

@@ -1600,26 +1592,23 @@ lorawan_status_t LoRaMac::prepare_frame(loramac_mhdr_t *machdr,
16001592
const uint8_t mac_commands_len = _mac_commands.get_mac_cmd_length();
16011593

16021594
if ((payload != NULL) && (_params.tx_buffer_len > 0)) {
1603-
if (_mac_commands.is_mac_command_in_next_tx() == true) {
1604-
if (mac_commands_len <= LORA_MAC_COMMAND_MAX_FOPTS_LENGTH) {
1605-
fctrl->bits.fopts_len += mac_commands_len;
1606-
1607-
// Update FCtrl field with new value of OptionsLength
1608-
_params.tx_buffer[0x05] = fctrl->value;
1609-
1610-
const uint8_t *buffer = _mac_commands.get_mac_commands_buffer();
1611-
for (i = 0; i < mac_commands_len; i++) {
1612-
_params.tx_buffer[pkt_header_len++] = buffer[i];
1613-
}
1614-
} else {
1615-
_params.tx_buffer_len = mac_commands_len;
1616-
payload = _mac_commands.get_mac_commands_buffer();
1617-
frame_port = 0;
1595+
if (mac_commands_len <= LORA_MAC_COMMAND_MAX_FOPTS_LENGTH) {
1596+
fctrl->bits.fopts_len += mac_commands_len;
1597+
1598+
// Update FCtrl field with new value of OptionsLength
1599+
_params.tx_buffer[0x05] = fctrl->value;
1600+
1601+
const uint8_t *buffer = _mac_commands.get_mac_commands_buffer();
1602+
for (i = 0; i < mac_commands_len; i++) {
1603+
_params.tx_buffer[pkt_header_len++] = buffer[i];
16181604
}
1605+
} else {
1606+
_params.tx_buffer_len = mac_commands_len;
1607+
payload = _mac_commands.get_mac_commands_buffer();
1608+
frame_port = 0;
16191609
}
16201610
} else {
1621-
if ((mac_commands_len > 0)
1622-
&& (_mac_commands.is_mac_command_in_next_tx() == true)) {
1611+
if (mac_commands_len > 0) {
16231612
_params.tx_buffer_len = mac_commands_len;
16241613
payload = _mac_commands.get_mac_commands_buffer();
16251614
frame_port = 0;
@@ -1636,7 +1625,6 @@ lorawan_status_t LoRaMac::prepare_frame(loramac_mhdr_t *machdr,
16361625
uint8_t *key = _params.keys.app_skey;
16371626
uint32_t key_length = sizeof(_params.keys.app_skey) * 8;
16381627
if (frame_port == 0) {
1639-
_mac_commands.clear_command_buffer();
16401628
key = _params.keys.nwk_skey;
16411629
key_length = sizeof(_params.keys.nwk_skey) * 8;
16421630
}
@@ -1813,7 +1801,6 @@ void LoRaMac::disconnect()
18131801

18141802
_mac_commands.clear_command_buffer();
18151803
_mac_commands.clear_repeat_buffer();
1816-
_mac_commands.clear_mac_commands_in_next_tx();
18171804

18181805
reset_mcps_confirmation();
18191806
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;
@@ -311,14 +294,6 @@ lorawan_status_t LoRaMacCommand::process_mac_commands(const uint8_t *payload, ui
311294
return ret_value;
312295
}
313296

314-
bool LoRaMacCommand::is_sticky_mac_command_pending()
315-
{
316-
if (mac_cmd_buf_idx_to_repeat > 0) {
317-
return true;
318-
}
319-
return false;
320-
}
321-
322297
int32_t LoRaMacCommand::cmd_buffer_remaining() const
323298
{
324299
// The maximum buffer length must take MAC commands to re-send into account.
@@ -337,7 +312,6 @@ lorawan_status_t LoRaMacCommand::add_link_check_req()
337312
mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_LINK_CHECK_REQ;
338313
// No payload for this command
339314
ret = LORAWAN_STATUS_OK;
340-
mac_cmd_in_next_tx = true;
341315
}
342316
return ret;
343317
}
@@ -349,7 +323,6 @@ lorawan_status_t LoRaMacCommand::add_link_adr_ans(uint8_t status)
349323
mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_LINK_ADR_ANS;
350324
mac_cmd_buffer[mac_cmd_buf_idx++] = status;
351325
ret = LORAWAN_STATUS_OK;
352-
mac_cmd_in_next_tx = true;
353326
}
354327
return ret;
355328
}
@@ -361,7 +334,6 @@ lorawan_status_t LoRaMacCommand::add_duty_cycle_ans()
361334
mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_DUTY_CYCLE_ANS;
362335
// No payload for this answer
363336
ret = LORAWAN_STATUS_OK;
364-
mac_cmd_in_next_tx = true;
365337
}
366338
return ret;
367339
}
@@ -376,7 +348,6 @@ lorawan_status_t LoRaMacCommand::add_rx_param_setup_ans(uint8_t status)
376348
// This is a sticky MAC command answer. Setup indication
377349
sticky_mac_cmd = true;
378350
ret = LORAWAN_STATUS_OK;
379-
mac_cmd_in_next_tx = true;
380351
}
381352
return ret;
382353
}
@@ -391,7 +362,6 @@ lorawan_status_t LoRaMacCommand::add_dev_status_ans(uint8_t battery, uint8_t mar
391362
mac_cmd_buffer[mac_cmd_buf_idx++] = battery;
392363
mac_cmd_buffer[mac_cmd_buf_idx++] = margin;
393364
ret = LORAWAN_STATUS_OK;
394-
mac_cmd_in_next_tx = true;
395365
}
396366
return ret;
397367
}
@@ -404,7 +374,6 @@ lorawan_status_t LoRaMacCommand::add_new_channel_ans(uint8_t status)
404374
// Status: Datarate range OK, Channel frequency OK
405375
mac_cmd_buffer[mac_cmd_buf_idx++] = status;
406376
ret = LORAWAN_STATUS_OK;
407-
mac_cmd_in_next_tx = true;
408377
}
409378
return ret;
410379
}
@@ -418,7 +387,6 @@ lorawan_status_t LoRaMacCommand::add_rx_timing_setup_ans()
418387
// This is a sticky MAC command answer. Setup indication
419388
sticky_mac_cmd = true;
420389
ret = LORAWAN_STATUS_OK;
421-
mac_cmd_in_next_tx = true;
422390
}
423391
return ret;
424392
}
@@ -430,7 +398,6 @@ lorawan_status_t LoRaMacCommand::add_tx_param_setup_ans()
430398
mac_cmd_buffer[mac_cmd_buf_idx++] = MOTE_MAC_TX_PARAM_SETUP_ANS;
431399
// No payload for this answer
432400
ret = LORAWAN_STATUS_OK;
433-
mac_cmd_in_next_tx = true;
434401
}
435402
return ret;
436403
}
@@ -445,7 +412,6 @@ lorawan_status_t LoRaMacCommand::add_dl_channel_ans(uint8_t status)
445412
// This is a sticky MAC command answer. Setup indication
446413
sticky_mac_cmd = true;
447414
ret = LORAWAN_STATUS_OK;
448-
mac_cmd_in_next_tx = true;
449415
}
450416
return ret;
451417
}

‎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)