Skip to content

Is this an error?! #250

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.

Sign up for GitHub

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

Jump to bottom
Closed
edogaldo opened this issue May 17, 2018 · 3 comments
Closed

Is this an error?! #250

edogaldo opened this issue May 17, 2018 · 3 comments
Assignees
@fpistm
Labels
answered question ❓ Usually converted as a discussion

Comments

@edogaldo
Copy link
Contributor

Arduino_Core_STM32/cores/arduino/stm32/clock.c

Line 99 in cf54e13

nb_loop = (((HAL_RCC_GetHCLKFreq() / 1000000)/5)*delay_us)+1; /* uS (divide by 4 because each loop take about 4 cycles including nop +1 is here to avoid delay of 0 */

In the comment you say "divide by 4" but in the code you divide by 5, is it an error?!
@fpistm
Copy link
Member

fpistm commented May 18, 2018

Hi, in fact it was done by @fprwi6labs for the old F0 core:
https://github.com/fpistm/Arduino_Core_STM32F0/blob/a97ed35b1252e4fb667f354019279216de518af1/cores/arduino/libstm32f0/clock.c#L178
He probably copy/paste from F4 implementation to F0 and adapt the code without update the comment.
When I've merged all the core in one I did not seen this.

@fpistm fpistm added the question ❓ Usually converted as a discussion label May 18, 2018
@fpistm fpistm self-assigned this May 18, 2018
@fpistm fpistm added the waiting feedback Further information is required label May 18, 2018
@edogaldo
Copy link
Contributor Author

edogaldo commented May 18, 2018 •
edited
Loading

Looking at the code, precision seems not to be a main concern: the function always waits for delay_us+1 us and also the function doesn't consider the time required to calculate (((HAL_RCC_GetHCLKFreq() / 1000000)/5)*delay_us)+1 and for function call (stack allocation/deallocation, jump & back).
So, as long as the only difference is the constant loop duration (4 vs 5) and the usage of subs.w vs sub, why not simplifying and factoring everything?

@fpistm
Copy link
Member

fpistm commented May 18, 2018

Yes, why not.
As I said this is linked to legacy code and probably some other clean/merge could be done elsewhere.

@fpistm fpistm closed this as completed in c5017df Sep 19, 2018
xC0000005 pushed a commit to xC0000005/Arduino_Core_STM32 that referenced this issue Nov 27, 2018
@fpistm @xC0000005
Remove non standard API
b08fee3 
delayInsideIT() code should have been in the method:
void GSM3SoftSerial::tunedDelay(uint16_t delay)
It was introduced as an API to support
built-in GSM library on Nucleo-L476RG and is not relevant for other.

Fix stm32duino#250

Signed-off-by: Frederic.Pillon <frederic.pillon@st.com>
benwaffle pushed a commit to benwaffle/Arduino_Core_STM32 that referenced this issue Apr 10, 2019
@fpistm
Remove non standard API
c447cc1 
delayInsideIT() code should have been in the method:
void GSM3SoftSerial::tunedDelay(uint16_t delay)
It was introduced as an API to support
built-in GSM library on Nucleo-L476RG and is not relevant for other.

Fix stm32duino#250

Signed-off-by: Frederic.Pillon <frederic.pillon@st.com>
@fpistm fpistm added answered and removed waiting feedback Further information is required labels Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fpistm fpistm

Labels
answered question ❓ Usually converted as a discussion
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@edogaldo @fpistm