Skip to content
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

Serial.flush modification #5293

Merged
merged 7 commits into from
Oct 8, 2019
Merged

Serial.flush modification #5293

merged 7 commits into from
Oct 8, 2019

Conversation

dsv19
Copy link
Contributor

@dsv19 dsv19 commented Oct 29, 2018

Added a few defines to extract width, parity and stop from uart mode in uart.c as suggested in the comments in #5094 and modified flush in HardwareSerial.cpp file.

@dsv19 dsv19 closed this Oct 29, 2018
@dsv19 dsv19 reopened this Oct 29, 2018
cores/esp8266/uart.c Outdated Show resolved Hide resolved
@d-a-v
Copy link
Collaborator

d-a-v commented Oct 29, 2018

CI fails: uart.h needs to be committed too.
Should we count the implicit start bit too (+1 in the result, 12 instead of 11) ?
Is it necessary to wait (to change to 11/12) when the returned value is 0 ?

@dsv19
Copy link
Contributor Author

dsv19 commented Oct 29, 2018

I'm not sure about the implicit start bit but it is not necessary to wait when the returned value is 0 since the HardwareSerial::flush function already handles a situation when uart is null.

@d-a-v
Copy link
Collaborator

d-a-v commented Oct 29, 2018

Start bit is always there, so IMHO 1 should be added to the tmp calculus.
Btw could you rename it for the readers, like bit_length or anything more explicit.
After a second read, you don't even need this variable, the value can be directly returned,
then there would be a comment to describe what's returned.

Copy link
Collaborator

@d-a-v d-a-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

cores/esp8266/uart.c Outdated Show resolved Hide resolved
@dsv19 dsv19 force-pushed the master branch 3 times, most recently from 49802d5 to 1ffa9fa Compare December 23, 2018 18:32
cores/esp8266/uart.c Outdated Show resolved Hide resolved
@earlephilhower earlephilhower added this to the 2.6.0 milestone Sep 28, 2019
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an improvement, and makes good sense.

@earlephilhower earlephilhower merged commit 8c3f1be into esp8266:master Oct 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants