Skip to content

Commit 1a0928e

Browse files
luc-githubluc-github
committed
Fix serial2socket module lock after a while
1 parent 6e43923 commit 1a0928e

File tree

3 files changed

+197
-47
lines changed

3 files changed

+197
-47
lines changed

‎esp3d/src/include/esp3d_version.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
#define _VERSION_ESP3D_H
2323

2424
// version and sources location
25-
#define FW_VERSION "3.0.0.6b1"
25+
#define FW_VERSION "3.0.0.7b1"
2626
#define REPOSITORY "https://github.com/luc-github/ESP3D/tree/3.0"
2727

2828
#endif //_VERSION_ESP3D_H

‎esp3d/src/modules/serial2socket/serial2socket.cpp

+188-40
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,67 @@
2323

2424
#if defined(ESP3DLIB_ENV) && COMMUNICATION_PROTOCOL == SOCKET_SERIAL
2525
#include <Arduino.h>
26+
#include "freertos/FreeRTOS.h"
27+
#include "freertos/semphr.h"
2628

2729
#include "../../core/esp3d_commands.h"
2830
#include "../../core/esp3d_message.h"
2931
#include "serial2socket.h"
3032

3133
Serial_2_Socket Serial2Socket;
3234

33-
Serial_2_Socket::Serial_2_Socket() { end(); }
34-
Serial_2_Socket::~Serial_2_Socket() { end(); }
35+
Serial_2_Socket::Serial_2_Socket() {
36+
_rxBufferMutex = (void*)xSemaphoreCreateMutex();
37+
if (_rxBufferMutex == NULL) {
38+
esp3d_log_e("Serial2Socket: Failed to create RX mutex");
39+
}
40+
41+
_txBufferMutex = (void*)xSemaphoreCreateMutex();
42+
if (_txBufferMutex == NULL) {
43+
esp3d_log_e("Serial2Socket: Failed to create TX mutex");
44+
}
45+
46+
end();
47+
}
48+
49+
Serial_2_Socket::~Serial_2_Socket() {
50+
if (_rxBufferMutex != NULL) {
51+
vSemaphoreDelete((SemaphoreHandle_t)_rxBufferMutex);
52+
_rxBufferMutex = NULL;
53+
}
54+
55+
if (_txBufferMutex != NULL) {
56+
vSemaphoreDelete((SemaphoreHandle_t)_txBufferMutex);
57+
_txBufferMutex = NULL;
58+
}
59+
60+
end();
61+
}
62+
3563
void Serial_2_Socket::begin(long speed) { end(); }
3664

3765
void Serial_2_Socket::enable(bool enable) { _started = enable; }
3866

3967
void Serial_2_Socket::pause(bool state) {
4068
_paused = state;
4169
if (_paused) {
42-
_TXbufferSize = 0;
43-
_RXbufferSize = 0;
44-
_RXbufferpos = 0;
70+
// Protect TX buffer access with mutex
71+
if (_txBufferMutex != NULL && xSemaphoreTake((SemaphoreHandle_t)_txBufferMutex, pdMS_TO_TICKS(100)) == pdTRUE) {
72+
_TXbufferSize = 0;
73+
xSemaphoreGive((SemaphoreHandle_t)_txBufferMutex);
74+
} else {
75+
_TXbufferSize = 0;
76+
}
77+
78+
// Protect RX buffer access with mutex
79+
if (_rxBufferMutex != NULL && xSemaphoreTake((SemaphoreHandle_t)_rxBufferMutex, pdMS_TO_TICKS(100)) == pdTRUE) {
80+
_RXbufferSize = 0;
81+
_RXbufferpos = 0;
82+
xSemaphoreGive((SemaphoreHandle_t)_rxBufferMutex);
83+
} else {
84+
_RXbufferSize = 0;
85+
_RXbufferpos = 0;
86+
}
4587
} else {
4688
_lastflush = millis();
4789
}
@@ -50,9 +92,24 @@ void Serial_2_Socket::pause(bool state) {
5092
bool Serial_2_Socket::isPaused() { return _paused; }
5193

5294
void Serial_2_Socket::end() {
53-
_TXbufferSize = 0;
54-
_RXbufferSize = 0;
55-
_RXbufferpos = 0;
95+
// Protect TX buffer access with mutex
96+
if (_txBufferMutex != NULL && xSemaphoreTake((SemaphoreHandle_t)_txBufferMutex, pdMS_TO_TICKS(100)) == pdTRUE) {
97+
_TXbufferSize = 0;
98+
xSemaphoreGive((SemaphoreHandle_t)_txBufferMutex);
99+
} else {
100+
_TXbufferSize = 0;
101+
}
102+
103+
// Protect RX buffer access with mutex
104+
if (_rxBufferMutex != NULL && xSemaphoreTake((SemaphoreHandle_t)_rxBufferMutex, pdMS_TO_TICKS(100)) == pdTRUE) {
105+
_RXbufferSize = 0;
106+
_RXbufferpos = 0;
107+
xSemaphoreGive((SemaphoreHandle_t)_rxBufferMutex);
108+
} else {
109+
_RXbufferSize = 0;
110+
_RXbufferpos = 0;
111+
}
112+
56113
_started = false;
57114
_paused = false;
58115
_lastflush = millis();
@@ -70,16 +127,32 @@ bool Serial_2_Socket::started() { return _started; }
70127
Serial_2_Socket::operator bool() const { return true; }
71128

72129
int Serial_2_Socket::available() {
73-
if (_paused) {
130+
if (_paused || !_started) {
74131
return 0;
75132
}
76-
return _RXbufferSize;
133+
if (_RXbufferSize == 0) {
134+
return 0;
135+
}
136+
137+
// Protect RX buffer access with mutex
138+
if (_rxBufferMutex != NULL && xSemaphoreTake((SemaphoreHandle_t)_rxBufferMutex, pdMS_TO_TICKS(100)) != pdTRUE) {
139+
esp3d_log_e("Serial2Socket: Failed to take mutex for available");
140+
return 0;
141+
}
142+
143+
int size = _RXbufferSize;
144+
145+
if (_rxBufferMutex != NULL) {
146+
xSemaphoreGive((SemaphoreHandle_t)_rxBufferMutex);
147+
}
148+
return size;
77149
}
78150

79151
size_t Serial_2_Socket::write(uint8_t c) {
80152
if (!_started || _paused) {
81153
return 1;
82154
}
155+
esp3d_log_d("Serial2Socket: write one char %c", c);
83156
return write(&c, 1);
84157
}
85158

@@ -88,39 +161,81 @@ size_t Serial_2_Socket::write(const uint8_t *buffer, size_t size) {
88161
esp3d_log("Serial2Socket: no data, not started or paused");
89162
return size;
90163
}
164+
165+
// Take TX buffer mutex once for the entire function
166+
if (_txBufferMutex != NULL && xSemaphoreTake((SemaphoreHandle_t)_txBufferMutex, pdMS_TO_TICKS(100)) != pdTRUE) {
167+
esp3d_log_e("Serial2Socket: Failed to take mutex for write");
168+
return 0;
169+
}
170+
91171
if (_TXbufferSize == 0) {
92172
_lastflush = millis();
93173
}
94-
// send full line
174+
175+
// Check if buffer is full and needs flushing
95176
if (_TXbufferSize + size > S2S_TXBUFFERSIZE) {
96-
flush();
177+
esp3d_log_d("Serial2Socket: buffer full, flush it");
178+
flush(false); // Use flush without mutex since we already have it
97179
}
98-
// need periodic check to force to flush in case of no end
180+
181+
// Add data to buffer and flush on newline
99182
for (int i = 0; i < size; i++) {
100183
_TXbuffer[_TXbufferSize] = buffer[i];
101184
_TXbufferSize++;
102185
if (buffer[i] == (const uint8_t)'\n') {
103-
esp3d_log("S2S: %s TXSize: %d", (const char *)_TXbuffer, _TXbufferSize);
104-
flush();
186+
esp3d_log_d("S2S: %s TXSize: %d", (const char *)_TXbuffer, _TXbufferSize);
187+
flush(false); // Use flush without mutex since we already have it
105188
}
106189
}
107-
handle_flush();
190+
191+
// Check if we need to flush based on time
192+
if (_TXbufferSize > 0 && ((millis() - _lastflush) > S2S_FLUSHTIMEOUT)) {
193+
flush(false); // Use flush without mutex since we already have it
194+
}
195+
196+
// Release the mutex at the end
197+
if (_txBufferMutex != NULL) {
198+
xSemaphoreGive((SemaphoreHandle_t)_txBufferMutex);
199+
}
200+
108201
return size;
109202
}
110203

111204
int Serial_2_Socket::peek(void) {
112-
if (_RXbufferSize > 0 && _started) {
113-
return _RXbuffer[_RXbufferpos];
114-
} else {
205+
esp3d_log_d("Serial2Socket: peek first of %d", _RXbufferSize);
206+
if (_RXbufferSize <= 0 || !_started) {
115207
return -1;
116208
}
209+
210+
// Protect RX buffer access with mutex
211+
if (_rxBufferMutex != NULL && xSemaphoreTake((SemaphoreHandle_t)_rxBufferMutex, pdMS_TO_TICKS(100)) != pdTRUE) {
212+
esp3d_log_e("Serial2Socket: Failed to take mutex for peek");
213+
return -1;
214+
}
215+
216+
uint8_t v = _RXbuffer[_RXbufferpos];
217+
218+
if (_rxBufferMutex != NULL) {
219+
xSemaphoreGive((SemaphoreHandle_t)_rxBufferMutex);
220+
}
221+
return v;
117222
}
118223

119-
bool Serial_2_Socket::push(const uint8_t *buffer, size_t size) {
224+
// Send data to socket output buffer
225+
bool Serial_2_Socket::push2RX(const uint8_t *buffer, size_t size) {
120226
if (buffer == NULL || size == 0 || !_started || _paused) {
121227
return false;
122228
}
229+
230+
// Protect RX buffer access with mutex
231+
if (_rxBufferMutex != NULL && xSemaphoreTake((SemaphoreHandle_t)_rxBufferMutex, pdMS_TO_TICKS(100)) != pdTRUE) {
232+
esp3d_log_e("Serial2Socket: cannot take mutex for push2RX");
233+
return false;
234+
}
235+
123236
int data_size = size;
237+
bool success = false;
238+
esp3d_log_d("Serial2Socket: pushing %d chars to buffer", data_size);
124239
if ((data_size + _RXbufferSize) <= S2S_RXBUFFERSIZE) {
125240
int current = _RXbufferpos + _RXbufferSize;
126241
if (current > S2S_RXBUFFERSIZE) {
@@ -134,24 +249,41 @@ bool Serial_2_Socket::push(const uint8_t *buffer, size_t size) {
134249
current++;
135250
}
136251
_RXbufferSize += size;
137-
_RXbuffer[current] = 0;
138-
return true;
252+
if (current < S2S_RXBUFFERSIZE) {
253+
_RXbuffer[current] = 0;
254+
}
255+
success = true;
139256
}
140-
return false;
257+
258+
if (_rxBufferMutex != NULL) {
259+
xSemaphoreGive((SemaphoreHandle_t)_rxBufferMutex);
260+
}
261+
return success;
141262
}
142263

143264
int Serial_2_Socket::read(void) {
144-
if (_RXbufferSize > 0 && _started && !_paused) {
145-
int v = _RXbuffer[_RXbufferpos];
146-
_RXbufferpos++;
147-
if (_RXbufferpos > (S2S_RXBUFFERSIZE - 1)) {
148-
_RXbufferpos = 0;
149-
}
150-
_RXbufferSize--;
151-
return v;
152-
} else {
265+
if (_RXbufferSize <= 0 || !_started || _paused) {
266+
return -1;
267+
}
268+
269+
// Protect RX buffer access with mutex
270+
if (_rxBufferMutex != NULL && xSemaphoreTake((SemaphoreHandle_t)_rxBufferMutex, pdMS_TO_TICKS(100)) != pdTRUE) {
271+
esp3d_log_e("Serial2Socket: Failed to take mutex for read");
153272
return -1;
154273
}
274+
275+
uint8_t v = _RXbuffer[_RXbufferpos];
276+
_RXbufferpos++;
277+
if (_RXbufferpos > (S2S_RXBUFFERSIZE - 1)) {
278+
_RXbufferpos = 0;
279+
}
280+
_RXbufferSize--;
281+
esp3d_log_d("Serial2Socket: read one char %c", v);
282+
283+
if (_rxBufferMutex != NULL) {
284+
xSemaphoreGive((SemaphoreHandle_t)_rxBufferMutex);
285+
}
286+
return v;
155287
}
156288

157289
void Serial_2_Socket::handle() { handle_flush(); }
@@ -161,27 +293,42 @@ void Serial_2_Socket::handle_flush() {
161293
if ((_TXbufferSize >= S2S_TXBUFFERSIZE) ||
162294
((millis() - _lastflush) > S2S_FLUSHTIMEOUT)) {
163295
esp3d_log("force socket flush");
164-
flush();
296+
flush(true);
165297
}
166298
}
167299
}
168-
void Serial_2_Socket::flush(void) {
300+
301+
void Serial_2_Socket::flush(bool useMutex) {
302+
303+
// Process buffer if there's data and we're not paused
169304
if (_TXbufferSize > 0 && _started && !_paused) {
305+
// Protect TX buffer access with mutex if requested
306+
if (useMutex && _txBufferMutex != NULL) {
307+
if (xSemaphoreTake((SemaphoreHandle_t)_txBufferMutex, pdMS_TO_TICKS(100)) != pdTRUE) {
308+
esp3d_log_e("Serial2Socket: Failed to take mutex for flush");
309+
return;
310+
}
311+
}
170312
ESP3DMessage *msg = esp3d_message_manager.newMsg(
171313
ESP3DClientType::socket_serial, ESP3DClientType::all_clients, _TXbuffer,
172314
_TXbufferSize, _auth);
173-
// dispatch command
315+
316+
// Reset buffer before processing
317+
_lastflush = millis();
318+
_TXbufferSize = 0;
319+
320+
// Release mutex if we took it
321+
if (useMutex && _txBufferMutex != NULL) {
322+
xSemaphoreGive((SemaphoreHandle_t)_txBufferMutex);
323+
}
324+
174325
if (msg) {
175326
// process command
176327
msg->type = ESP3DMessageType::unique;
177328
esp3d_commands.process(msg);
178329
} else {
179330
esp3d_log_e("Cannot create message");
180331
}
181-
// refresh timout
182-
_lastflush = millis();
183-
// reset buffer
184-
_TXbufferSize = 0;
185332
}
186333
}
187334

@@ -191,7 +338,8 @@ bool Serial_2_Socket::dispatch(ESP3DMessage *message) {
191338
return false;
192339
}
193340
if (message->size > 0 && message->data) {
194-
if (!push(message->data, message->size)) {
341+
esp3d_log_d("Serial2Socket: dispatch message %d", message->size);
342+
if (!push2RX(message->data, message->size)) {
195343
esp3d_log_e("Serial2Socket: cannot push all data");
196344
return false;
197345
}

‎esp3d/src/modules/serial2socket/serial2socket.h

+8-6
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
#include <Print.h>
2525

2626
#include "../authentication/authentication_level_types.h"
27-
#define S2S_TXBUFFERSIZE 1200
28-
#define S2S_RXBUFFERSIZE 128
27+
#define S2S_TXBUFFERSIZE 512
28+
#define S2S_RXBUFFERSIZE 256
2929
#define S2S_FLUSHTIMEOUT 500
3030

3131
class ESP3DMessage; // forward declaration
@@ -51,8 +51,8 @@ class Serial_2_Socket : public Stream {
5151
int available();
5252
int peek(void);
5353
int read(void);
54-
bool push(const uint8_t *buffer, size_t size);
55-
void flush(void);
54+
bool push2RX(const uint8_t *buffer, size_t size);
55+
void flush(bool useMutex = true);
5656
void handle_flush();
5757
void handle();
5858
operator bool() const;
@@ -63,10 +63,12 @@ class Serial_2_Socket : public Stream {
6363
bool _started;
6464
bool _paused;
6565
ESP3DAuthenticationLevel _auth;
66+
void* _rxBufferMutex; // Opaque pointer for RX buffer mutex
67+
void* _txBufferMutex; // Opaque pointer for TX buffer mutex
6668
uint32_t _lastflush;
67-
uint8_t _TXbuffer[S2S_TXBUFFERSIZE];
69+
uint8_t _TXbuffer[S2S_TXBUFFERSIZE+1];
6870
uint16_t _TXbufferSize;
69-
uint8_t _RXbuffer[S2S_RXBUFFERSIZE];
71+
uint8_t _RXbuffer[S2S_RXBUFFERSIZE+1];
7072
uint16_t _RXbufferSize;
7173
uint16_t _RXbufferpos;
7274
};

0 commit comments

Comments
 (0)