From ef264b67f3af27357a1a52b834e224a29cd509d8 Mon Sep 17 00:00:00 2001 From: TMRh20 Date: Mon, 1 Jul 2024 06:38:53 -0600 Subject: [PATCH 1/5] Fixes: rexmit & hold - Fix: re-transmit mechanism - this was not working if waiting for an ack due to the `hold` variable being set. - Fix: `hold` variable was not being set or unset where it should be - Remove out_pos & dataPos variables from userdata struct, replace with data_pos variable - Add attribute_packed to the userdata struct since it is now mis-aligned - Now only transmit one RF24Network payload instead of a retry, since the rexmit mechanism is now working --- RF24Client.cpp | 83 ++++++++++++++++++++++++++---------------------- RF24Client.h | 6 ++-- RF24Ethernet.cpp | 13 +++----- 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/RF24Client.cpp b/RF24Client.cpp index 49f2648..7b89bea 100644 --- a/RF24Client.cpp +++ b/RF24Client.cpp @@ -196,16 +196,16 @@ size_t RF24Client::_write(uip_userdata_t* u, const uint8_t* buf, size_t size) if (u && !(u->state & (UIP_CLIENT_CLOSE | UIP_CLIENT_REMOTECLOSED)) && u->state & (UIP_CLIENT_CONNECTED)) { - if (u->out_pos + payloadSize > UIP_TCP_MSS || u->hold) + if (u->data_pos + payloadSize > UIP_TCP_MSS || u->hold) { goto test2; } - IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(); Serial.print(millis()); Serial.print(F(" UIPClient.write: writePacket(")); Serial.print(u->packets_out); Serial.print(F(") pos: ")); Serial.print(u->out_pos); Serial.print(F(", buf[")); Serial.print(size - total_written); Serial.print(F("]: '")); Serial.write((uint8_t*)buf + total_written, payloadSize); Serial.println(F("'"));); + IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(); Serial.print(millis()); Serial.print(F(" UIPClient.write: writePacket(")); Serial.print(u->packets_out); Serial.print(F(") pos: ")); Serial.print(u->data_pos); Serial.print(F(", buf[")); Serial.print(size - total_written); Serial.print(F("]: '")); Serial.write((uint8_t*)buf + total_written, payloadSize); Serial.println(F("'"));); - memcpy(u->myData + u->out_pos, buf + total_written, payloadSize); + memcpy(u->myData + u->data_pos, buf + total_written, payloadSize); u->packets_out = 1; - u->out_pos += payloadSize; + u->data_pos += payloadSize; total_written += payloadSize; @@ -217,7 +217,8 @@ size_t RF24Client::_write(uip_userdata_t* u, const uint8_t* buf, size_t size) // RF24EthernetClass::tick(); goto test2; } - return u->out_pos; + u->hold = false; + return u->data_pos; } u->hold = false; return -1; @@ -278,18 +279,15 @@ void serialip_appcall(void) #if UIP_CONNECTION_TIMEOUT > 0 u->connectTimer = millis(); #endif + u->hold = (u->data_pos = (u->windowOpened = (u->packets_out = false))); - if (u->sent) - { - u->hold = (u->out_pos = (u->windowOpened = (u->packets_out = false))); - } if (uip_len && !(u->state & (UIP_CLIENT_CLOSE | UIP_CLIENT_REMOTECLOSED))) { uip_stop(); u->state &= ~UIP_CLIENT_RESTART; u->windowOpened = false; u->restartTime = millis(); - memcpy(&u->myData[u->dataPos + u->dataCnt], uip_appdata, uip_datalen()); + memcpy(&u->myData[u->data_pos + u->dataCnt], uip_appdata, uip_datalen()); u->dataCnt += uip_datalen(); u->packets_in = 1; @@ -327,7 +325,7 @@ void serialip_appcall(void) { IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(); Serial.print(millis()); Serial.println(F(" UIPClient uip_acked"));); u->state &= ~UIP_CLIENT_RESTART; - u->hold = (u->out_pos = (u->windowOpened = (u->packets_out = false))); + u->hold = (u->data_pos = (u->windowOpened = (u->packets_out = false))); u->restartTime = millis(); #if UIP_CONNECTION_TIMEOUT > 0 u->connectTimer = millis(); @@ -337,39 +335,46 @@ void serialip_appcall(void) /*******Polling**********/ if (uip_poll() || uip_rexmit()) { + if (uip_rexmit()) { + IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(F("ReXmit, Len: "));); + IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(u->data_pos)); + uip_len = u->data_pos; + uip_send(u->myData, u->data_pos); + u->hold = true; + goto finish; + } // IF_RF24ETHERNET_DEBUG_CLIENT( Serial.println(); Serial.println(F("UIPClient uip_poll")); ); - if (u->packets_out != 0) + if (u->packets_out != 0 && !u->hold) { - uip_len = u->out_pos; - uip_send(u->myData, u->out_pos); + uip_len = u->data_pos; + uip_send(u->myData, u->data_pos); u->hold = true; - u->sent = true; goto finish; } - else - // Restart mechanism to keep connections going - // Only call this if the TCP window has already been re-opened, the connection is being polled, but no data - // has been acked - if (!(u->state & (UIP_CLIENT_CLOSE | UIP_CLIENT_REMOTECLOSED))) - { - if (u->windowOpened == true && u->state & UIP_CLIENT_RESTART && millis() - u->restartTime > u->restartInterval) - { - u->restartTime = millis(); + // Restart mechanism to keep connections going + // Only call this if the TCP window has already been re-opened, the connection is being polled, but no data + // has been acked + if (!(u->state & (UIP_CLIENT_CLOSE | UIP_CLIENT_REMOTECLOSED))) + { + + if (u->windowOpened == true && u->state & UIP_CLIENT_RESTART && millis() - u->restartTime > u->restartInterval) + { + u->restartTime = millis(); #if defined RF24ETHERNET_DEBUG_CLIENT || defined ETH_DEBUG_L1 - Serial.println(); - Serial.print(millis()); + Serial.println(); + Serial.print(millis()); #if UIP_CONNECTION_TIMEOUT > 0 - Serial.print(F(" UIPClient Re-Open TCP Window, time remaining before abort: ")); - Serial.println(UIP_CONNECTION_TIMEOUT - (millis() - u->connectTimer)); + Serial.print(F(" UIPClient Re-Open TCP Window, time remaining before abort: ")); + Serial.println(UIP_CONNECTION_TIMEOUT - (millis() - u->connectTimer)); #endif #endif - u->restartInterval += 500; - u->restartInterval = rf24_min(u->restartInterval, 7000); - uip_restart(); - } + u->restartInterval += 500; + u->restartInterval = rf24_min(u->restartInterval, 7000); + uip_restart(); } + } } /*******Close**********/ @@ -423,9 +428,11 @@ uip_userdata_t* RF24Client::_allocateData() data->packets_in = 0; data->packets_out = 0; data->dataCnt = 0; - data->dataPos = 0; - data->out_pos = 0; + //data->dataPos = 0; + data->data_pos = 0; data->hold = 0; + data->restartTime = millis(); + data->restartInterval = 5000; #if (UIP_CONNECTION_TIMEOUT > 0) data->connectTimer = millis(); data->connectTimeout = UIP_CONNECTION_TIMEOUT; @@ -483,15 +490,15 @@ int RF24Client::read(uint8_t* buf, size_t size) } size = rf24_min(data->dataCnt, size); - memcpy(buf, &data->myData[data->dataPos], size); + memcpy(buf, &data->myData[data->data_pos], size); data->dataCnt -= size; - data->dataPos += size; + data->data_pos += size; if (!data->dataCnt) { data->packets_in = 0; - data->dataPos = 0; + data->data_pos = 0; if (uip_stopped(&uip_conns[data->state & UIP_CLIENT_SOCKETS]) && !(data->state & (UIP_CLIENT_CLOSE | UIP_CLIENT_REMOTECLOSED))) { @@ -536,7 +543,7 @@ int RF24Client::peek() { if (available()) { - return data->myData[data->dataPos]; + return data->myData[data->data_pos]; } return -1; } diff --git a/RF24Client.h b/RF24Client.h index 238fe5d..595f41b 100644 --- a/RF24Client.h +++ b/RF24Client.h @@ -50,16 +50,14 @@ typedef struct * Data structure for holding per connection data * @warning This is used internally and should not be accessed directly by users */ -typedef struct +typedef __attribute__((__packed__)) struct { bool hold; - bool sent; bool packets_in; bool packets_out; bool windowOpened; uint8_t state; - uint16_t out_pos; - uint16_t dataPos; + uint16_t data_pos; uint16_t dataCnt; #if UIP_CLIENT_TIMER >= 0 uint32_t timer; diff --git a/RF24Ethernet.cpp b/RF24Ethernet.cpp index 90fb47f..cafbb5b 100644 --- a/RF24Ethernet.cpp +++ b/RF24Ethernet.cpp @@ -317,16 +317,13 @@ void RF24EthernetClass::network_send() bool ok = RF24Ethernet.network.write(headerOut, uip_buf, uip_len); - if (!ok) { - ok = RF24Ethernet.network.write(headerOut, uip_buf, uip_len); #if defined ETH_DEBUG_L1 || defined ETH_DEBUG_L2 - if (!ok) { - Serial.println(); - Serial.print(millis()); - Serial.println(F(" *** RF24Ethernet Network Write Fail ***")); - } -#endif + if (!ok) { + Serial.println(); + Serial.print(millis()); + Serial.println(F(" *** RF24Ethernet Network Write Fail ***")); } +#endif #if defined ETH_DEBUG_L2 if (ok) { From 52722dd338ce4e78e4dfbe9876b8a5cceba63896 Mon Sep 17 00:00:00 2001 From: TMRh20 Date: Mon, 1 Jul 2024 06:54:23 -0600 Subject: [PATCH 2/5] Additions to last commit --- RF24Client.h | 2 +- RF24Ethernet.cpp | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/RF24Client.h b/RF24Client.h index 595f41b..e00b3b9 100644 --- a/RF24Client.h +++ b/RF24Client.h @@ -50,7 +50,7 @@ typedef struct * Data structure for holding per connection data * @warning This is used internally and should not be accessed directly by users */ -typedef __attribute__((__packed__)) struct +typedef struct __attribute__((__packed__)) { bool hold; bool packets_in; diff --git a/RF24Ethernet.cpp b/RF24Ethernet.cpp index cafbb5b..e231644 100644 --- a/RF24Ethernet.cpp +++ b/RF24Ethernet.cpp @@ -315,14 +315,15 @@ void RF24EthernetClass::network_send() { RF24NetworkHeader headerOut(00, EXTERNAL_DATA_TYPE); - bool ok = RF24Ethernet.network.write(headerOut, uip_buf, uip_len); - #if defined ETH_DEBUG_L1 || defined ETH_DEBUG_L2 + bool ok = RF24Ethernet.network.write(headerOut, uip_buf, uip_len); if (!ok) { Serial.println(); Serial.print(millis()); Serial.println(F(" *** RF24Ethernet Network Write Fail ***")); } +#else + RF24Ethernet.network.write(headerOut, uip_buf, uip_len); #endif #if defined ETH_DEBUG_L2 From 5ea785af800fc9b66c95e59a456917e2f89a43c3 Mon Sep 17 00:00:00 2001 From: TMRh20 Date: Mon, 1 Jul 2024 06:55:21 -0600 Subject: [PATCH 3/5] Update RF24Ethernet.cpp Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- RF24Ethernet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RF24Ethernet.cpp b/RF24Ethernet.cpp index e231644..4792a57 100644 --- a/RF24Ethernet.cpp +++ b/RF24Ethernet.cpp @@ -323,7 +323,7 @@ void RF24EthernetClass::network_send() Serial.println(F(" *** RF24Ethernet Network Write Fail ***")); } #else - RF24Ethernet.network.write(headerOut, uip_buf, uip_len); + RF24Ethernet.network.write(headerOut, uip_buf, uip_len); #endif #if defined ETH_DEBUG_L2 From 0662744436ebbbea8015b73342b14b7e92983af4 Mon Sep 17 00:00:00 2001 From: TMRh20 Date: Mon, 1 Jul 2024 07:03:41 -0600 Subject: [PATCH 4/5] println to print --- RF24Client.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RF24Client.cpp b/RF24Client.cpp index 7b89bea..128f277 100644 --- a/RF24Client.cpp +++ b/RF24Client.cpp @@ -336,7 +336,7 @@ void serialip_appcall(void) if (uip_poll() || uip_rexmit()) { if (uip_rexmit()) { - IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(F("ReXmit, Len: "));); + IF_RF24ETHERNET_DEBUG_CLIENT(Serial.print(F("ReXmit, Len: "));); IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(u->data_pos)); uip_len = u->data_pos; uip_send(u->myData, u->data_pos); From 2c5ed330629dcf3b691f7a2d8599016ead05ded9 Mon Sep 17 00:00:00 2001 From: TMRh20 Date: Mon, 1 Jul 2024 08:45:51 -0600 Subject: [PATCH 5/5] Partially revert prev change - Fix one thing, break another... need to separate incoming and outgoing data counters --- RF24Client.cpp | 38 +++++++++++++++++++------------------- RF24Client.h | 3 ++- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/RF24Client.cpp b/RF24Client.cpp index 128f277..2b6f8f7 100644 --- a/RF24Client.cpp +++ b/RF24Client.cpp @@ -196,16 +196,16 @@ size_t RF24Client::_write(uip_userdata_t* u, const uint8_t* buf, size_t size) if (u && !(u->state & (UIP_CLIENT_CLOSE | UIP_CLIENT_REMOTECLOSED)) && u->state & (UIP_CLIENT_CONNECTED)) { - if (u->data_pos + payloadSize > UIP_TCP_MSS || u->hold) + if (u->out_pos + payloadSize > UIP_TCP_MSS || u->hold) { goto test2; } - IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(); Serial.print(millis()); Serial.print(F(" UIPClient.write: writePacket(")); Serial.print(u->packets_out); Serial.print(F(") pos: ")); Serial.print(u->data_pos); Serial.print(F(", buf[")); Serial.print(size - total_written); Serial.print(F("]: '")); Serial.write((uint8_t*)buf + total_written, payloadSize); Serial.println(F("'"));); + IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(); Serial.print(millis()); Serial.print(F(" UIPClient.write: writePacket(")); Serial.print(u->packets_out); Serial.print(F(") pos: ")); Serial.print(u->out_pos); Serial.print(F(", buf[")); Serial.print(size - total_written); Serial.print(F("]: '")); Serial.write((uint8_t*)buf + total_written, payloadSize); Serial.println(F("'"));); - memcpy(u->myData + u->data_pos, buf + total_written, payloadSize); + memcpy(u->myData + u->out_pos, buf + total_written, payloadSize); u->packets_out = 1; - u->data_pos += payloadSize; + u->out_pos += payloadSize; total_written += payloadSize; @@ -218,7 +218,7 @@ size_t RF24Client::_write(uip_userdata_t* u, const uint8_t* buf, size_t size) goto test2; } u->hold = false; - return u->data_pos; + return u->out_pos; } u->hold = false; return -1; @@ -279,7 +279,7 @@ void serialip_appcall(void) #if UIP_CONNECTION_TIMEOUT > 0 u->connectTimer = millis(); #endif - u->hold = (u->data_pos = (u->windowOpened = (u->packets_out = false))); + u->hold = (u->out_pos = (u->windowOpened = (u->packets_out = false))); if (uip_len && !(u->state & (UIP_CLIENT_CLOSE | UIP_CLIENT_REMOTECLOSED))) { @@ -287,7 +287,7 @@ void serialip_appcall(void) u->state &= ~UIP_CLIENT_RESTART; u->windowOpened = false; u->restartTime = millis(); - memcpy(&u->myData[u->data_pos + u->dataCnt], uip_appdata, uip_datalen()); + memcpy(&u->myData[u->in_pos + u->dataCnt], uip_appdata, uip_datalen()); u->dataCnt += uip_datalen(); u->packets_in = 1; @@ -325,7 +325,7 @@ void serialip_appcall(void) { IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(); Serial.print(millis()); Serial.println(F(" UIPClient uip_acked"));); u->state &= ~UIP_CLIENT_RESTART; - u->hold = (u->data_pos = (u->windowOpened = (u->packets_out = false))); + u->hold = (u->out_pos = (u->windowOpened = (u->packets_out = false))); u->restartTime = millis(); #if UIP_CONNECTION_TIMEOUT > 0 u->connectTimer = millis(); @@ -337,9 +337,9 @@ void serialip_appcall(void) { if (uip_rexmit()) { IF_RF24ETHERNET_DEBUG_CLIENT(Serial.print(F("ReXmit, Len: "));); - IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(u->data_pos)); - uip_len = u->data_pos; - uip_send(u->myData, u->data_pos); + IF_RF24ETHERNET_DEBUG_CLIENT(Serial.println(u->out_pos)); + uip_len = u->out_pos; + uip_send(u->myData, u->out_pos); u->hold = true; goto finish; } @@ -347,8 +347,8 @@ void serialip_appcall(void) if (u->packets_out != 0 && !u->hold) { - uip_len = u->data_pos; - uip_send(u->myData, u->data_pos); + uip_len = u->out_pos; + uip_send(u->myData, u->out_pos); u->hold = true; goto finish; } @@ -428,8 +428,8 @@ uip_userdata_t* RF24Client::_allocateData() data->packets_in = 0; data->packets_out = 0; data->dataCnt = 0; - //data->dataPos = 0; - data->data_pos = 0; + data->in_pos = 0; + data->out_pos = 0; data->hold = 0; data->restartTime = millis(); data->restartInterval = 5000; @@ -490,15 +490,15 @@ int RF24Client::read(uint8_t* buf, size_t size) } size = rf24_min(data->dataCnt, size); - memcpy(buf, &data->myData[data->data_pos], size); + memcpy(buf, &data->myData[data->in_pos], size); data->dataCnt -= size; - data->data_pos += size; + data->in_pos += size; if (!data->dataCnt) { data->packets_in = 0; - data->data_pos = 0; + data->in_pos = 0; if (uip_stopped(&uip_conns[data->state & UIP_CLIENT_SOCKETS]) && !(data->state & (UIP_CLIENT_CLOSE | UIP_CLIENT_REMOTECLOSED))) { @@ -543,7 +543,7 @@ int RF24Client::peek() { if (available()) { - return data->myData[data->data_pos]; + return data->myData[data->in_pos]; } return -1; } diff --git a/RF24Client.h b/RF24Client.h index e00b3b9..cb90f2f 100644 --- a/RF24Client.h +++ b/RF24Client.h @@ -57,7 +57,8 @@ typedef struct __attribute__((__packed__)) bool packets_out; bool windowOpened; uint8_t state; - uint16_t data_pos; + uint16_t in_pos; + uint16_t out_pos; uint16_t dataCnt; #if UIP_CLIENT_TIMER >= 0 uint32_t timer;