Skip to content

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