Skip to content

Commit 7a15132

Browse files
CapnBrypkendall64
CapnBry
and
pkendall64
authored
Stop calling MSP.resetState and replace a message in transit (ExpressLRS#1145)
* Stop calling MSP.resetState * Update unit test to take account of reduced retry count before resync * Next state is SENDING after SEND_NEXT * Use RESYNC_THEN_SEND to abort a subborn transfer Co-authored-by: Paul Kendall <[email protected]>
1 parent 98ccefd commit 7a15132

4 files changed

Lines changed: 67 additions & 15 deletions

File tree

‎src/lib/StubbornSender/stubborn_sender.cpp‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,20 +9,24 @@ StubbornSender::StubbornSender(uint8_t maxPackageIndex)
99

1010
void StubbornSender::ResetState()
1111
{
12-
data = 0;
12+
data = nullptr;
1313
bytesPerCall = 1;
1414
currentOffset = 0;
1515
currentPackage = 0;
1616
length = 0;
1717
waitUntilTelemetryConfirm = true;
1818
waitCount = 0;
19-
maxWaitCount = 1000;
19+
// 80 corresponds to UpdateTelemetryRate(ANY, 2, 1), which is what the TX uses in boost mode
20+
maxWaitCount = 80;
2021
senderState = SENDER_IDLE;
2122
}
2223

24+
/***
25+
* Queues a message to send, will abort the current message if one is currently being transmitted
26+
***/
2327
void StubbornSender::SetDataToTransmit(uint8_t lengthToTransmit, uint8_t* dataToTransmit, uint8_t bytesPerCall)
2428
{
25-
if (senderState != SENDER_IDLE || lengthToTransmit / bytesPerCall >= maxPackageIndex)
29+
if (lengthToTransmit / bytesPerCall >= maxPackageIndex)
2630
{
2731
return;
2832
}
@@ -33,7 +37,7 @@ void StubbornSender::SetDataToTransmit(uint8_t lengthToTransmit, uint8_t* dataTo
3337
currentPackage = 1;
3438
waitCount = 0;
3539
this->bytesPerCall = bytesPerCall;
36-
senderState = SENDING;
40+
senderState = (senderState == SENDER_IDLE) ? SENDING : RESYNC_THEN_SEND;
3741
}
3842

3943
bool StubbornSender::IsActive()
@@ -46,6 +50,7 @@ void StubbornSender::GetCurrentPayload(uint8_t *packageIndex, uint8_t *count, ui
4650
switch (senderState)
4751
{
4852
case RESYNC:
53+
case RESYNC_THEN_SEND:
4954
*packageIndex = maxPackageIndex;
5055
*count = 0;
5156
*currentData = 0;
@@ -103,13 +108,14 @@ void StubbornSender::ConfirmCurrentPayload(bool telemetryConfirmValue)
103108
{
104109
nextSenderState = WAIT_UNTIL_NEXT_CONFIRM;
105110
}
106-
107111
break;
112+
108113
case RESYNC:
114+
case RESYNC_THEN_SEND:
109115
case WAIT_UNTIL_NEXT_CONFIRM:
110116
if (telemetryConfirmValue == waitUntilTelemetryConfirm)
111117
{
112-
nextSenderState = SENDER_IDLE;
118+
nextSenderState = (senderState == RESYNC_THEN_SEND) ? SENDING : SENDER_IDLE;
113119
waitUntilTelemetryConfirm = !telemetryConfirmValue;
114120
}
115121
// switch to resync if tx does not confirm value fast enough
@@ -122,10 +128,9 @@ void StubbornSender::ConfirmCurrentPayload(bool telemetryConfirmValue)
122128
nextSenderState = RESYNC;
123129
}
124130
}
125-
126131
break;
132+
127133
case SENDER_IDLE:
128-
case SEND_NEXT:
129134
break;
130135
}
131136

‎src/lib/StubbornSender/stubborn_sender.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
typedef enum {
99
SENDER_IDLE = 0,
1010
SENDING,
11-
SEND_NEXT,
1211
WAIT_UNTIL_NEXT_CONFIRM,
13-
RESYNC
12+
RESYNC,
13+
RESYNC_THEN_SEND, // perform a RESYNC then go to SENDING
1414
} stubborn_sender_state_s;
1515

1616
class StubbornSender

‎src/src/tx_main.cpp‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ void DynamicPower_Update()
218218
POWERMGNT.incPower();
219219
}
220220
if (avg_rssi > rssi_dec_threshold) {
221-
DBGLN("Power decrease");
221+
DBGVLN("Power decrease");
222222
POWERMGNT.decPower();
223223
}
224224

@@ -685,7 +685,6 @@ void BackpackBinding()
685685
static void SendRxWiFiOverMSP()
686686
{
687687
MSPDataPackage[0] = MSP_ELRS_SET_RX_WIFI_MODE;
688-
MspSender.ResetState();
689688
MspSender.SetDataToTransmit(1, MSPDataPackage, ELRS_MSP_BYTES_PER_CALL);
690689
}
691690

@@ -770,7 +769,6 @@ void SendUIDOverMSP()
770769
MSPDataPackage[2] = MasterUID[3];
771770
MSPDataPackage[3] = MasterUID[4];
772771
MSPDataPackage[4] = MasterUID[5];
773-
MspSender.ResetState();
774772
BindingSendCount = 0;
775773
MspSender.SetDataToTransmit(5, MSPDataPackage, ELRS_MSP_BYTES_PER_CALL);
776774
}
@@ -835,7 +833,6 @@ void ExitBindingMode()
835833

836834
InBindingMode = false;
837835

838-
MspSender.ResetState();
839836
SetRFLinkRate(config.GetRate()); //return to original rate
840837

841838
DBGLN("Exiting binding mode");

‎src/test/stubborn_native/test_stubborn.cpp‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <unity.h>
66
#include <iostream>
77
#include <bitset>
8+
#include "helpers.h"
89

910
StubbornSender sender(ELRS_TELEMETRY_MAX_PACKAGES);
1011
StubbornReceiver receiver(ELRS_TELEMETRY_MAX_PACKAGES);
@@ -284,7 +285,7 @@ void test_stubborn_link_sends_data_until_confirmation(void)
284285
receiver.ResetState();
285286
receiver.SetDataToReceive(sizeof(buffer), buffer, 1);
286287

287-
for(int i = 0; i < 100; i++)
288+
for(int i = 0; i < sender.GetMaxPacketsBeforeResync(); i++)
288289
{
289290
sender.GetCurrentPayload(&packageIndex, &maxLength, &data);
290291
TEST_ASSERT_EQUAL(1, packageIndex);
@@ -328,6 +329,54 @@ void test_stubborn_link_multiple_packages(void)
328329

329330
}
330331

332+
static void test_stubborn_link_resync_then_send(void)
333+
{
334+
uint8_t testSequence1[] = {1,2,3,4,5,6,7,8,9,10};
335+
uint8_t testSequence2[] = {11,12,13,14,15,16,17,18,19,20};
336+
uint8_t buffer[100];
337+
uint8_t *data;
338+
uint8_t maxLength;
339+
uint8_t packageIndex;
340+
341+
receiver.ResetState();
342+
receiver.SetDataToReceive(sizeof(buffer), buffer, 1);
343+
344+
sender.ResetState();
345+
sender.SetDataToTransmit(sizeof(testSequence1), testSequence1, 1);
346+
347+
// send and confirm two packages
348+
sender.GetCurrentPayload(&packageIndex, &maxLength, &data);
349+
receiver.ReceiveData(packageIndex, data);
350+
sender.ConfirmCurrentPayload(receiver.GetCurrentConfirm());
351+
352+
sender.GetCurrentPayload(&packageIndex, &maxLength, &data);
353+
receiver.ReceiveData(packageIndex, data);
354+
sender.ConfirmCurrentPayload(receiver.GetCurrentConfirm());
355+
356+
// Abort the transfer by changing the payload
357+
sender.SetDataToTransmit(sizeof(testSequence2), testSequence2, 1);
358+
359+
// Send next packet, which should be a RESYNC
360+
sender.GetCurrentPayload(&packageIndex, &maxLength, &data);
361+
TEST_ASSERT_EQUAL(ELRS_TELEMETRY_MAX_PACKAGES, packageIndex);
362+
receiver.ReceiveData(packageIndex, data);
363+
sender.ConfirmCurrentPayload(receiver.GetCurrentConfirm());
364+
365+
// Complete the transfer
366+
int maxSends = ELRS_TELEMETRY_MAX_PACKAGES + 1;
367+
while (!receiver.HasFinishedData() && maxSends)
368+
{
369+
sender.GetCurrentPayload(&packageIndex, &maxLength, &data);
370+
receiver.ReceiveData(packageIndex, data);
371+
sender.ConfirmCurrentPayload(receiver.GetCurrentConfirm());
372+
}
373+
374+
// Should not have exhausted all the sends to get the package to go
375+
TEST_ASSERT_NOT_EQUAL(0, maxSends);
376+
// Make sure the second package was received, not the first
377+
TEST_ASSERT_EQUAL_UINT8_ARRAY(testSequence2, buffer, ARRAY_SIZE(testSequence2));
378+
}
379+
331380
int main(int argc, char **argv)
332381
{
333382
UNITY_BEGIN();
@@ -341,6 +390,7 @@ int main(int argc, char **argv)
341390
RUN_TEST(test_stubborn_link_sends_data_until_confirmation);
342391
RUN_TEST(test_stubborn_link_multiple_packages);
343392
RUN_TEST(test_stubborn_link_resyncs_during_last_confirm);
393+
RUN_TEST(test_stubborn_link_resync_then_send);
344394
UNITY_END();
345395

346396
return 0;

0 commit comments

Comments
 (0)