-
Notifications
You must be signed in to change notification settings - Fork 3k
ESP8266: Fix serial flow control inconsistency on reconnect #15240
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: Fix serial flow control inconsistency on reconnect #15240
Conversation
@ccli8, thank you for your changes. |
* @return true if started | ||
*/ | ||
bool stop_uart_hw_flow_ctrl(); | ||
bool stop_uart_hw_flow_ctrl(bool board_only); |
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.
As this is public method, this would be breaking, wouldn't it?
What if we add overload adding boolean argument, we could deprecate the old method if it is necessary.
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.
How about C++ default argument:
bool stop_uart_hw_flow_ctrl(bool board_only = false);
The call to stop_uart_hw_flow_ctrl()
without board_only
specified will translate to stop_uart_hw_flow_ctrl(false)
for compatibility.
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.
Also was considering this, https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rf-default-args - lets go with the default parameter
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.
Updated to go with default parameter
The commit will address the test failure of connectivity-netsocket-tests-tests-network-interface. In the test, serial channel will break with the sequence: ESP8266Interface::connect() > disconnect() > connect() In the first connect, both board's and ESP8266's serial flow control default to disabled, and then enabled. In the second connect, board's serial flow control keeps enabled but ESP8266's resets to disabled, causing inconsistency between two ends. The approach: Explicitly disable board's serial flow control on re-power or reset in (re-)connect
08e9732
to
0d2de99
Compare
CI started |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
Summary of changes
The PR tries to address the test failure of
connectivity-netsocket-tests-tests-network-interface
. In the test, serial channel will break with the sequence:In the first connect, both board's and ESP8266's serial flow control default to disabled, and then enabled. However, in the second connect, board's serial flow control keeps enabled but ESP8266's resets to disabled, causing inconsistency between two ends.
The approach is explicitly disable board's serial flow control on ESP8266 re-power or reset in (re-)connect.
Pull request type
Test results