Skip to content

Commit 051a711

Browse files
andreagilardoniandreagilardoni
committed
fixup! [WIP] Improving parse function in modem class
1 parent 40a326a commit 051a711

File tree

2 files changed

+147
-130
lines changed

2 files changed

+147
-130
lines changed

‎libraries/WiFiS3/src/Modem.cpp

Lines changed: 139 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -49,21 +49,24 @@ bool ModemClass::passthrough(const uint8_t *data, size_t size) {
4949
/* -------------------------------------------------------------------------- */
5050
_serial->write(data,size);
5151

52-
std::string tmp, data_res; // FIXME
53-
bool res = buf_read(tmp, data_res);
54-
55-
// if(_serial_debug && _debug_level >= 2) {
56-
// _serial_debug->print(" ANSWER (passthrough): ");
57-
// _serial_debug->println(data_res.c_str());
58-
// if(res) {
59-
// _serial_debug->println(" Result: OK");
60-
// }
61-
// else {
62-
// _serial_debug->println(" Result: FAILED");
63-
// }
64-
// }
52+
std::string tmp, data_res; // FIXME I don't always have a command prompt provided
53+
auto res = buf_read(tmp, data_res);
6554

66-
return res;
55+
if(_serial_debug && _debug_level >= 2) {
56+
_serial_debug->print(" ANSWER (passthrough): ");
57+
_serial_debug->println(data_res.c_str());
58+
if(res == Ok) {
59+
_serial_debug->println(" Result: OK");
60+
} else if(res = Error) {
61+
_serial_debug->println(" Result: ERROR");
62+
} else if(res = Timeout) {
63+
_serial_debug->println(" Result: TIMEOUT");
64+
} else {
65+
_serial_debug->println(" Result: ParseError");
66+
}
67+
}
68+
69+
return res == Ok;
6770
}
6871

6972
/* -------------------------------------------------------------------------- */
@@ -102,66 +105,77 @@ bool ModemClass::write(const string &prompt, string &data_res, const char * fmt,
102105
}
103106

104107
_serial->write(tx_buff,strlen((char *)tx_buff));
105-
return buf_read(prompt, data_res);;
106-
}
107-
108-
109-
typedef enum {
110-
IDLE,
111-
WAIT_FOR_SIZE,
112-
WAIT_FOR_DATA
113-
} ReadBySizeSt_t;
108+
auto res = buf_read(prompt, data_res);
114109

110+
if(_serial_debug) {
111+
_serial_debug->print(" ANSWER: ");
112+
_serial_debug->println(data_res.c_str());
113+
if(res == Ok) {
114+
_serial_debug->println(" Result: OK");
115+
} else if(res = Error) {
116+
_serial_debug->println(" Result: ERROR");
117+
} else if(res = Timeout) {
118+
_serial_debug->println(" Result: TIMEOUT");
119+
} else {
120+
_serial_debug->println(" Result: ParseError");
121+
}
122+
}
115123

116-
/* -------------------------------------------------------------------------- */
117-
bool ModemClass::read_by_size_finished(string &rx) {
118-
/* -------------------------------------------------------------------------- */
119-
bool rv = false;
120-
return rv;
124+
return res == Ok;
121125
}
122126

123127

124-
enum class at_parse_state_t {
125-
Begin,
126-
Cmd,
127-
Data,
128-
Sized,
129-
ResWaitLF,
130-
Res,
131-
Error,
132-
ParseError,
133-
Ok,
134-
Completed,
135-
};
136-
137128
/* -------------------------------------------------------------------------- */
138-
bool ModemClass::buf_read(const string &prompt, string &data_res) {
129+
ModemClass::ParseResult ModemClass::buf_read(const string &prompt, string &data_res) {
139130
/* -------------------------------------------------------------------------- */
140-
bool res = false;
141-
142-
if(_serial_debug && _debug_level >= 1) {
143-
_serial_debug->print("RAW: ");
144-
}
131+
/*
132+
* This function implements as FSM that parses basic AT command responses
133+
* The expected syntax should match the following regex
134+
* - (?:\+(\w+)[:=][ ]?(\w*))?(?:\r\n)?(ERROR\r\n|OK\r\n)
135+
* + "ERROR<CR><LF>" "OK<CR><LF>"
136+
* + "+COMMAND: <CR><LF>OK<CR><LF>"
137+
* + "+COMMAND: <CR><LF>ERROR<CR><LF>"
138+
* + "+COMMAND: 1231231<CR><LF>OK<CR><LF>" (NOTE only one parameter supported)
139+
* + "+COMMAND: 1231231<CR><LF>ERROR<CR><LF>" (NOTE only one parameter supported)
140+
* - custom sized response:
141+
* + "+COMMAND: 4| 123OK<CR><LF>"
142+
*/
143+
enum class at_parse_state_t {
144+
Begin,
145+
Cmd,
146+
Data,
147+
Sized,
148+
ResWaitLF,
149+
Res,
150+
Error,
151+
ParseError,
152+
Ok,
153+
Completed,
154+
};
145155

146156
at_parse_state_t state = at_parse_state_t::Begin;
147157
std::string commandName;
148158

159+
ModemClass::ParseResult res = Error;
149160
unsigned int sized_read_size = 0;
150161
unsigned int sized_read_count = 0;
151-
unsigned int result_parse = 1;
162+
unsigned int result_parse = 0;
163+
164+
165+
if(_serial_debug && _debug_level >= 1) {
166+
_serial_debug->print("RAW: ");
167+
}
152168

153169
unsigned long start_time = millis();
154170
while(state != at_parse_state_t::Completed) {
155171
if(millis() - start_time > _timeout) {
156-
_serial_debug->println("Timeout");
172+
res = Timeout;
157173
break;
158174
}
159175

160-
while(_serial->available()) {
176+
while(_serial->available() && state != at_parse_state_t::Completed) {
161177
char c = _serial->read();
162178
if(_serial_debug && _debug_level >= 1) {
163-
// _serial_debug->print(c);
164-
165179
if(c == '\n') {
166180
_serial_debug->print("<LF>");
167181
} else if (c == '\r') {
@@ -175,137 +189,144 @@ bool ModemClass::buf_read(const string &prompt, string &data_res) {
175189
} else {
176190
_serial_debug->print(c);
177191
}
178-
// _serial_debug->print((int)state);
179-
// _serial_debug->print('\n');
180192
}
181193

182194
switch(state) {
183195
case at_parse_state_t::Begin:
196+
/*
197+
* In this state we wait for a '+' character, which will mark the beginning of a response
198+
* or the status response code "ERROR<CR><LF>" or "OK<CR><LF>"
199+
* we need to consume the available buffer if it doesn't match the expected response,
200+
* in order to avoiding dealing with previous responses which were not parsed successfully
201+
*/
202+
184203
if(c == '+') {
185-
_serial_debug->println("\nNew State: Command parse");
186204
state = at_parse_state_t::Cmd;
187-
} else if(c == RESULT_OK[0]) { // OK response
188-
_serial_debug->println("\nNew State: OK");
189-
205+
} else if(c == RESULT_OK[result_parse]) { // FIXME this should also account for following characters
190206
state = at_parse_state_t::Ok;
191-
} else if(c == RESULT_ERROR[0]) { // Error response
192-
_serial_debug->println("\nNew State: Error");
193-
207+
} else if(c == RESULT_ERROR[result_parse]) { // FIXME this should also account for following characters
194208
state = at_parse_state_t::Error;
195209
} else {
196-
state = at_parse_state_t::ParseError;
210+
data_res = "";
197211
}
212+
// if we uncomment this we can force strict response matching
213+
// else {
214+
// state = at_parse_state_t::ParseError;
215+
// }
198216

199217
break;
200218
case at_parse_state_t::Cmd:
219+
/*
220+
* In this state we parse the command prompt and wait for either ':' or '=' characters
221+
* in order to go the next state
222+
*/
201223

202224
if(c == ':' || c == '=') {
203225
// TODO verify command is matching prompt
204-
// _serial_debug->print("\"\nData: \"");
205-
_serial_debug->println("\nNew State: Data parse");
206-
207226
state = at_parse_state_t::Data;
208-
} else { // avoid the first ' ' space
209-
// _serial_debug->print(c);
210-
commandName += c; // TODO verify command name
227+
} else {
228+
commandName += c;
211229
}
212230

213231
break;
214232
case at_parse_state_t::Data:
215-
if(c == '|') { // sized read, the previous parameter is the length
216-
_serial_debug->print("\nNew State: Sized ");
233+
/*
234+
* In this state we parse response parameters and push them into data_res
235+
* in case multiple parameters separated by ',' are sent, they will be present in data_res
236+
* - if we encounter <CR> we need to wait for <LF>
237+
* - if we encounter <LF> we need to parse the response status
238+
* - if we encounter '|', the next token will contain binary sized data, the current value in
239+
* in data_res contains the length of the next token
240+
*/
217241

242+
if(c == '|') { // sized read, the previous parameter is the length
218243
state = at_parse_state_t::Sized;
244+
219245
sized_read_size = atoi(data_res.c_str());
220-
_serial_debug->println(sized_read_size);
221246
data_res.clear();
222-
} else if(c == '\r' && data_res == "OK") { // FIXME
223-
result_parse = 3;
224-
225-
_serial_debug->println("\nNew State: OK");
226-
state = at_parse_state_t::Ok;
227-
} else if(c == '\r' && data_res == "ERROR") { // FIXME
228-
result_parse = 6;
229-
_serial_debug->println("\nNew State: ERROR");
230-
state = at_parse_state_t::Error;
231247
} else if(c == '\r') {
232-
_serial_debug->println("\nNew State: ResWaitLF");
233248
state = at_parse_state_t::ResWaitLF;
234249
} else if(c == '\n') {
235-
_serial_debug->println("\nNew State: RES");
236-
237250
state = at_parse_state_t::Res;
238-
} else if(c != ' ') { // FIXME should space be eliminated?
251+
} else if(c != ' ') { // FIXME should we keep the space?
239252
data_res += c;
240253
}
241254

242255
break;
243256
case at_parse_state_t::Sized:
257+
/*
258+
* In this state we collect exactly sized_read_size characters into data_res
259+
* when we consume all of them we go into Result parse state, where we supposedly
260+
* wait for 'OK'
261+
*/
244262
data_res += c;
245263

246-
_serial_debug->print(sized_read_count);
247-
_serial_debug->print(", ");
248-
_serial_debug->print(sized_read_size);
249-
250264
if(++sized_read_count == sized_read_size) {
251-
_serial_debug->println("\nNew State: RES");
252265
state = at_parse_state_t::Res;
253-
// do not consume another character, but parse the current one instead
254-
continue;
255266
}
256267
break;
257268
case at_parse_state_t::ResWaitLF:
258269
if(c == '\n') {
259-
_serial_debug->println("\nNew State: RES");
260270
state = at_parse_state_t::Res;
261-
} else {
262-
_serial_debug->println("A");
263-
state = at_parse_state_t::ParseError;
264271
}
265-
break;
272+
273+
/*
274+
* break is volountary not present, to cover for cases where the response status is in the
275+
* following form: '...<CR>OK<CR><LF>' '<CR>ERROR<CR><LF>'
276+
*/
266277
case at_parse_state_t::Res:
267-
if(c == RESULT_OK[0]) { // OK response
268-
_serial_debug->println("\nNew State: OK");
278+
/*
279+
* In this state we wait for either an 'O' or an 'E', in order to get an 'OK<CR><LF>'
280+
* or 'ERROR<CR><LF>'
281+
*/
269282

283+
if(data_res == "OK") { // FIXME make it constant
284+
res = Ok;
285+
state = at_parse_state_t::Completed;
286+
} else if(data_res == "ERROR") { // FIXME make it constant
287+
res = Error;
288+
state = at_parse_state_t::Completed;
289+
} if(c == RESULT_OK[0]) { // OK response
290+
res = Ok;
270291
state = at_parse_state_t::Ok;
271292
} else if(c == RESULT_ERROR[0]) { // Error response
272-
_serial_debug->println("\nNew State: Error");
273-
293+
res = Error;
274294
state = at_parse_state_t::Error;
275295
}
296+
// if we uncomment this we can force strict response matching
276297
// else {
277298
// state = at_parse_state_t::ParseError;
278-
// // do not consume another character
279-
// continue;
280299
// }
281300
break;
282-
case at_parse_state_t::Ok: // Conusme the buffer abd verify that it contains "K"
283-
res = true;
301+
case at_parse_state_t::Ok:
302+
/*
303+
* In this state we want to match the exact 'K<CR><LF>' response
304+
*/
284305

285-
if(c != RESULT_OK[result_parse++]) {
286-
_serial_debug->println("C");
306+
if(c != RESULT_OK[++result_parse]) {
307+
_serial_debug->println("pippo");
287308
state = at_parse_state_t::ParseError;
288309
}
289310

290-
if(result_parse >= strlen(RESULT_OK)) {
311+
if(result_parse == strlen(RESULT_OK)) {
291312
state = at_parse_state_t::Completed;
292313
}
293314
break;
294-
case at_parse_state_t::Error: // Conusme the buffer and verify that it contains "RROR"
295-
res = false;
296-
297-
if(c != RESULT_ERROR[result_parse++]) {
298-
_serial_debug->println("D");
315+
case at_parse_state_t::Error:
316+
/*
317+
* In this state we want to match the exact 'RROR<CR><LF>' response
318+
*/
299319

320+
if(c != RESULT_ERROR[++result_parse]) {
300321
state = at_parse_state_t::ParseError;
301322
}
302323

303-
if(result_parse >= strlen(RESULT_ERROR)) {
324+
if(result_parse == strlen(RESULT_ERROR)) {
304325
state = at_parse_state_t::Completed;
305326
}
306327
break;
307328
case at_parse_state_t::ParseError:
308-
_serial_debug->println("ParseError");
329+
res = ParseError; // FIXME we need to differentiate between failed status and parse error
309330
state = at_parse_state_t::Completed;
310331
break;
311332
case at_parse_state_t::Completed:
@@ -314,27 +335,16 @@ bool ModemClass::buf_read(const string &prompt, string &data_res) {
314335
}
315336
}
316337

317-
if(trim_results) {
318-
trim(data_res);
319-
}
320-
trim_results = true;
338+
// if(trim_results) { // we don't need this, since we are skipping spaces in Data state
339+
// trim(data_res);
340+
// }
341+
// trim_results = true;
321342

322343
if(_serial_debug && _debug_level >= 1) {
323344
_serial_debug->print("<-RAW END");
324345
_serial_debug->println();
325346
}
326347

327-
if(_serial_debug) {
328-
_serial_debug->print(" ANSWER: ");
329-
_serial_debug->println(data_res.c_str());
330-
if(res) {
331-
_serial_debug->println(" Result: OK");
332-
}
333-
else {
334-
_serial_debug->println(" Result: FAILED");
335-
}
336-
}
337-
338348
return res;
339349
}
340350

0 commit comments

Comments
 (0)