Base the 'msleep' function on MillisElapsed

[Acssiohm]

I just tried to apply the "TODO", but I'm not sure if it's what was expected.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
7 out of 9 committers have signed the CLA.

✅ GabrielNumworks
✅ EmilieNumworks
✅ HugoNumworks
✅ M4xi1m3
✅ Acssiohm
✅ thePeras
✅ RedGl0w
❌ RobertaNumWorks
❌ MartijnNumworks
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[RedGl0w]

Good idea, however here are other changes to do / questions :

• In the timing.h config files, LoopsPerMillisecond should be removed
• commits should be squashed
• I added a coding style comment
• Since the sysTick frequency is updated too when the board clock is updated, why not making these change to usleep too ?
  the systick would need to be changed : we wouldn't want it to be called every 1ms, but every 1us (if it is possible).

[Acssiohm]

For the last point , I think it's a good idea too ( but I don't know if it will affect the calculator speed, if so, I don't know if it's worth it )
For the first one, I don't think it's necessary because it's an interesting value that can be used later by others, and if it is not used, the compiler will delete it anyway
Thanks for the coding style comment
Finally I don't know if you see it , but there is still a bug, that I don't really understand

[Acssiohm]

Also, I know I should squash the commits, but , first, I still try to understand the bug, and second I don't know how to do that

[RedGl0w]

For the last point , I think it's a good idea too ( but I don't know if it will affect the calculator speed, if so, I don't know if it's worth it )

The usleep is only used by drivers, so even if there is an important speed difference, it won't be a problem for the end user (if the drivers still works).

For the first one, I don't think it's necessary because it's an interesting value that can be used later by others, and if it is not used, the compiler will delete it anyway

Even if it is an interessing value, it shouldn't be used anymore, and if we want to find it, the git history still has the value.

Finally I don't know if you see it , but there is still a bug, that I don't really understand

Yep I was looking about it.

[Acssiohm]

The usleep is only used by drivers, so even if there is an important speed difference, it won't be a problem for the end user (if the drivers still

I am not talking about usleep, I'm talking about systick :

the systick would need to be changed : we wouldn't want it to be called every 1ms, but every 1us (if it is possible).

because we will be calling this function a thousand times more often

[RedGl0w]

I am not talking about usleep, I'm talking about systick :

Yep, but you can call the systick function 1000 times more often only when usleeping, and call it every 1ms on msleep
Not sure it is a super good solution though, maybe writing in asm the usleep function would be better (and assert that the compilator doesn't optimize differently)

[Acssiohm]

The problem is that it is not us who call the sysTick function, it's an interrupt, when it's configured, you can't change it.

[RedGl0w]

The problem is that it is not us who call the sysTick function, it's an interrupt, when it's configured, you can't change it.

you can set the frequency of systick indeed (and it's configurated : when you go from high speed to low speed, when msleeping, the systick frequency is changed, in order to still tick every 1ms)

[Acssiohm]

you can set the frequency of systick indeed (and it's configurated : when you go from high speed to low speed, when msleeping, the systick frequency is changed, in order to still tick every 1ms)

Indeed ... you're right, I didn't see your message before sending mine (deleted now) :

My bad, if a did understand the datasheet, there is maybe a systick reload value register, that you can change

and, indeed, the register ( that I was talking about ) is modified in setClockFrequency to compensate ( like you said ) .
But, what you can't change is the function called by the interrupt : isr_systick() , and it's a problem because is increment the MillisElapsed but if we make it called 1000 times more often, MillisElapsed will be incremented every microsecond, and the variable will have no sense, sometimes incremented every millisecond and sometimes every microsecond !
So you have maybe 2 solutions :

• you can verify the frequency in the isr_systick() function, to sometimes increment MicrosElapsed ( the variable will need to be renamed ) by 1 , and sometimes by 1000 ( and in millis() you return MicrosElapsed/1000 )
• or you can just name the variable like sysTicksElapsed and use it in usleep like microseconds, and in msleep like milliseconds , elapsed, by changing the frequency of systicks ...

[github-actions]

	.text	.rodata	.bss	.data
Base	686 156 bytes	274 039 bytes	180 720 bytes	1 368 bytes
Head	686 164 bytes	274 068 bytes	180 720 bytes	1 368 bytes
	+ 8 bytes	+ 29 bytes	+ 0 bytes	+ 0 bytes
	+ 0.0 %	+ 0.0 %	+ 0.0 %	+ 0.0 %

[Acssiohm]

I don't understand why this problem appears, but I found where is the reason, and how to fix it.
It worked but I had to modify the LD file to make MillisElapsed one of the not discarded symbols and I don't know if it's recommended to do that.
But I wonder why MillisElapsed is discarded ( it's an important variable ) and why they could use it in millis() ( and use millis() in apps ) but we cannot use it in msleep(), if it's discarded it should be discarded everywhere , isn't it ?

[github-actions]

	.text	.rodata	.bss	.data
Base	686 156 bytes	274 039 bytes	180 720 bytes	1 368 bytes
Head	686 156 bytes	274 039 bytes	180 720 bytes	1 368 bytes
	+ 0 bytes	+ 0 bytes	+ 0 bytes	+ 0 bytes
	+ 0.0 %	+ 0.0 %	+ 0.0 %	+ 0.0 %

[Acssiohm]

I'll just comment the last changes :

• I removed the constant ( just commented it for now ) LoopsPerMillisecond like you said
• I made a .bss section for the variable we are using ( because it needs to be in a .bss section, to be initialized correctly )
• And finally I moved endTime to the top in case, changing the frequency takes time and removed volatile because endTime is not volatile ( it is not changed by an outside function, during an interrupt for example )

[Acssiohm]

So you have maybe 2 solutions :

• you can verify the frequency in the isr_systick() function, to sometimes increment MicrosElapsed ( the variable will need to be renamed ) by 1 , and sometimes by 1000 ( and in millis() you return MicrosElapsed/1000 )
• or you can just name the variable like sysTicksElapsed and use it in usleep like microseconds, and in msleep like milliseconds , elapsed, by changing the frequency of systicks ...

Wich one do you think is better ?
( the first require verifying the frequency at every sysTick, the second is a make millis() impossible ... )

[Acssiohm]

What did just happen ?? hundreds of commits appeared before my first one in this Pull Request and also a lot of conflicts came out !

[adriweb]

master was updated. You can rebase your commit.