ESP8266 driver changes to use ATCmdParser #46
Conversation
ESP8266/ESP8266.cpp
Outdated
| @@ -49,7 +52,7 @@ bool ESP8266::startup(int mode) | |||
| && _parser.send("AT+CIPMUX=1") | |||
| && _parser.recv("OK"); | |||
|
|
|||
| _parser.oob("+IPD", this, &ESP8266::_packet_handler); | |||
| _parser.oob("+IPD", Callback<void()>(this, &ESP8266::_packet_handler)); | |||
There was a problem hiding this comment.
This could just be:
_parser.oob("+IPD", callback(this, &ESP8266::_packet_handler));
ESP8266/ESP8266.cpp
Outdated
| } | ||
|
|
||
| bool ESP8266::readable() | ||
| { | ||
| return _serial.readable(); | ||
| if ( _serial.poll( POLLIN ) & POLLIN ) return true; | ||
| return false; |
There was a problem hiding this comment.
couldn't this be return _serial.readable()?
There was a problem hiding this comment.
Hi @geky UARTSerial does private inheritance of SerialBase and prevents us from calling readable/writable. In addition, I think poll call implementation is little better in that it avoids spinning.
There was a problem hiding this comment.
You would need to be calling UARTSerial::readable(), not SerialBase::readable. You need to know if there's anything in the buffer, not the hardware.
Presumably you're avoiding ARMmbed/mbed-os#5066? That could otherwise be avoided with by saying _serial.FileHandle::readable().
I don't believe these functions are ever used anyway?
There was a problem hiding this comment.
See your point, yes we want to check the buffer and not the hardware fifo/buffer, I'll change it to using _serial.FileHandle::readable() and serial.FileHandle::writable().
ESP8266/ESP8266.cpp
Outdated
| } | ||
|
|
||
| bool ESP8266::writeable() | ||
| { | ||
| return _serial.writeable(); | ||
| if ( _serial.poll( POLLOUT ) & POLLOUT ) return true; | ||
| return false; |
There was a problem hiding this comment.
couldn't this be return _serial.writeable()?
| @@ -26,7 +26,7 @@ using namespace utest::v1; | |||
|
|
|||
| namespace { | |||
| // Test connection information | |||
| const char *HTTP_SERVER_NAME = "developer.mbed.org"; | |||
| const char *HTTP_SERVER_NAME = "os.mbed.com"; | |||
ESP8266/ESP8266.cpp
Outdated
| @@ -17,11 +17,14 @@ | |||
| #include "ESP8266.h" | |||
|
|
|||
| ESP8266::ESP8266(PinName tx, PinName rx, bool debug) | |||
| : _serial(tx, rx, 1024), _parser(_serial) | |||
| , _packets(0), _packets_end(&_packets) | |||
| : _serial(tx, rx, 1024), | |||
There was a problem hiding this comment.
Unless someone has changed UARTSerial while I wasn't looking, it doesn't let you set the buffer size (because of CircularBuffer) - 1024 would be baud rate. :(
There was a problem hiding this comment.
Good catch, Actually I do call set_baud() in the constructor so the correct baud is still set. Ill fix this.
|
Should this PR remove the ATParser lib file to prevent it from being pulled in with the driver? https://github.com/ARMmbed/esp8266-driver/blob/master/ESP8266/ATParser.lib |
|
Well yes, the current plan is we are going to add a comment in ESP8266 driver repo to say that ATParser is deprecated. This is to get any comments developers might have before we really remove it. |
geky
force-pushed
the
sen_ESP8266ATCmdParser
branch
2 times, most recently
from
cc96bfb to
3422018
Compare
geky
force-pushed
the
sen_ESP8266ATCmdParser
branch
from
3422018 to
65a58dd
Compare
ARM Internal REF: IOTMORF-1232