Ensures ES8266 firmware version up to date #23
Conversation
If ESP8266 firmware version is less that v2.0.0, then we print a debug message and return an error from the connect function.
sarahmarshy
force-pushed
the
firmware-version-error
branch
from
72db791 to
a7016f7
Compare
| * Check firmware version of ESP8266 | ||
| * | ||
| * @return true only if ESP8266 firmware > v2 | ||
| */ |
There was a problem hiding this comment.
Hmm, thoughts on returning the firmware version and checking at a higher level? Maybe int get_firmware_version()?
ESP8266Interface.cpp
Outdated
| { | ||
| _esp.setTimeout(ESP8266_MISC_TIMEOUT); | ||
|
|
||
| if(!_esp.check_firmware_version()) { |
There was a problem hiding this comment.
This needs to occur after the esp8266 is reset (in ESP8266::startup) otherwise the esp8266 may be in a bad error state an not respond.
ESP8266Interface.cpp
Outdated
| _esp.setTimeout(ESP8266_MISC_TIMEOUT); | ||
|
|
||
| if(!_esp.check_firmware_version()) { | ||
| debug("ERROR: Firmware incompatible with this driver.\ |
There was a problem hiding this comment.
Looks like a reasonable error message, I would add something at the front that indicates the esp8266 is throwing an error (maybe "ESP8266: ERROR: blablabalabla").
There was a problem hiding this comment.
Also, I would omit the specific version, maybe just say "most recent version". Otherwise maintaining that number may become annoying.
There was a problem hiding this comment.
Or actually, have the current version defined as ESP8266_VERSION, then use "%d" in the debug statement so we only have to update one number (and maybe use that define for the version check).
ESP8266Interface.h
Outdated
| #define ESP8266_INTERFACE_H | ||
|
|
||
| #include "mbed.h" | ||
| #include "mbed_debug.h" |
There was a problem hiding this comment.
Yes. Compiliation fails otherwise, apparently it is not included in mbed.h.
There was a problem hiding this comment.
Hm ok, in that case can you actually move it into the cpp file for now?
It should probably be in mbed.h if you want to make a pr against mbed-os
ESP8266/ESP8266.cpp
Outdated
| if(_parser.recv("SDK version:%d", &version) && _parser.recv("OK")) { | ||
| return version; | ||
| } | ||
| else { |
There was a problem hiding this comment.
style: else on same line as previous bracket, and space after if:
if (_parser.recv("SDK version:%d", &version) && _parser.recv("OK")) {
return version;
} else {
sarahmarshy
force-pushed
the
firmware-version-error
branch
from
ae673ec to
64e5e78
Compare
ESP8266Interface.cpp
Outdated
| { | ||
| _esp.setTimeout(ESP8266_CONNECT_TIMEOUT); | ||
|
|
||
| if(!_esp.reset()) { |
ESP8266Interface.cpp
Outdated
|
|
||
| _esp.setTimeout(ESP8266_MISC_TIMEOUT); | ||
|
|
||
| if(_esp.get_firmware_version() != ESP8266_VERSION) { |
There was a problem hiding this comment.
style if (_esp.get_firmware_version() != ESP8266_VERSION) {
ESP8266Interface.cpp
Outdated
| return NSAPI_ERROR_DEVICE_ERROR; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
if you update this pr, can you remove this whitespace change?
Firmware version pinned to macro. Check for compatibility occurs at Interface level.
sarahmarshy
force-pushed
the
firmware-version-error
branch
from
64e5e78 to
530ab25
Compare
geky
left a comment
geky
left a comment
There was a problem hiding this comment.
Looks good, thinks for updating the style 👍
|
+2 to that :) |
3 participants
If ESP8266 firmware version is less that v2.0.0, then we print a debug message and return an error from the connect function. The message directs users to the wiki page for updating the firmware.