-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
esp8266 _oob_* : recv() brought back where it was needed #11541
Conversation
@dmaziec1, thank you for your changes. |
@VeijoPesonen , your feedback, as usually, will be appreciated :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify OK on #11537
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok but could you also add a comment to ATCmdParser.hpp about scanf and its limitations. The fact that it won't consider whitespace as part of the string it tries to match. Information source, @kjbracey-arm . An issue should also be created that there is a need to combine scanf's and recv's implementations in a way that scanf is just the one which gives up immeadiatelly after it doesn't find a match. Then it would also work as expected in a case like this where there are multiple digits at the end of a line.
@VeijoPesonen , regarding the comment that you would like us to add... do you mean something like this, but moved to the doxygen header's documentation? Could you please clarify a bit further on what the bug report should aim at? |
Yes, that should work.
|
f05d9cc
to
cb7ff53
Compare
platform/ATCmdParser.h
Outdated
@@ -271,6 +271,9 @@ class ATCmdParser : private NonCopyable<ATCmdParser> { | |||
|
|||
/** | |||
* Direct scanf on underlying stream | |||
* This function does not itself match whitespace in its format string, so \n is not significant to it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix indentation. I would say that ESP8266 and ATCmdParser changes should be done on separate commits.
cb7ff53
to
e84e098
Compare
A separate PR was created (and approved) for documentation update. |
CI started |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Description
As reported in: #11537 in
esp8266::_oob_*
recv()
is brought back wherescanf()
fails.scanf()
immediately returns after finding one digit of the number, whilerecv()
matches the whole number.Pull request type
Reviewers
@ccli8
@michalpasztamobica
@0xc0170
Release Notes