Skip to content

Fix issue #478. #480

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

Merged
merged 1 commit into from
Mar 25, 2019
Merged

Fix issue #478. #480

merged 1 commit into from
Mar 25, 2019

Conversation

ghent360
Copy link
Contributor

@ghent360 ghent360 commented Mar 25, 2019

This PR fixes/implements the following bugs/features

In CDC_connected the core reads the value of the global variable transmitStart twice. This is a problem because the variable is modified inside ISR and the value can change between the two reads. It can lead to calculating the wrong value for transmitTime which can lead to loss of data.

This proposed fix would save the value of transmitStart in a local variable, so we read the value only once.

Fixes #478

@fpistm fpistm self-requested a review March 25, 2019 07:59
@fpistm fpistm added bug 🐛 Something isn't working fix 🩹 Bug fix labels Mar 25, 2019
@fpistm fpistm merged commit dd3d946 into stm32duino:master Mar 25, 2019
@ktand
Copy link
Contributor

ktand commented Mar 25, 2019

@ghent360 @fpistm The fix is incorrect.

The line:

transmitTime = HAL_GetTick() - transmitStart;

should be

transmitTime = HAL_GetTick() - transmitTime;

otherwise you'll be reading transmitStart twice, as before.

@ghent360
Copy link
Contributor Author

@ktand You are absolutely correct.

@ghent360
Copy link
Contributor Author

I'm the dunce of the week. Can't make a 3 line code fix right.

@fpistm
Copy link
Member

fpistm commented Mar 26, 2019

Thanks @ktand
Now, fixed in master.
I think I read too much code 🙄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working fix 🩹 Bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

USBSerial loosing characters on Linux
3 participants