Issue #2958 Correct calculation of subticks being returned from port_get_raw_ticks() for esp32s2 #3020
Conversation
updating from adafruit
Issue #2958 . Correct calculation of usec back to ticks in port_get_raw_ticks().
ports/esp32s2/supervisor/port.c
Outdated
| @@ -118,11 +118,12 @@ uint32_t port_get_saved_word(void) { | |||
| uint64_t port_get_raw_ticks(uint8_t* subticks) { | |||
| struct timeval tv_now; | |||
| gettimeofday(&tv_now, NULL); | |||
| uint64_t all_subticks = (uint64_t)tv_now.tv_usec / 32768; | |||
| // convert usec back to ticks | |||
| uint64_t all_subticks = (uint64_t)tv_now.tv_usec / 1024; | |||
There was a problem hiding this comment.
I don't think this is correct because all_subticks should range from 0 to 32767 which would be achieved by dividing 1000000 by 30.517. We can approximate this with the change below. Line 125 should be preserved.
| uint64_t all_subticks = (uint64_t)tv_now.tv_usec / 1024; | |
| uint64_t all_subticks = (uint64_t)(tv_now.tv_usec * 2) / 71; |
There was a problem hiding this comment.
With tv_usec having a value between 1 and 1 million, multiplying it by 2 and dividing by 71 give us values from 1 to 28168. Since a tick is 1/1024th of a second, that could add to the return value by up to 27 seconds. It seems correct to me to multiply seconds by 1024, and add subticks of 1000000 divided by 1024, which is what the original fix did. That would give us subticks of 1 to 1000, which when added to seconds*1024 would be the right number of ticks.
Testing of the proposed modified fix showed times much greater than the call to sleep() requested.
There was a problem hiding this comment.
How are you measuring the sleep duration? I think there is more going on with FreeRTOS that makes it harder to measure.
The all_subticks should be divided by 32 before being added to seconds * 1024 because one tick is 32 subticks. (Most boards have 32.768khz crystals which is 1024 * 32)
There was a problem hiding this comment.
I used the script above to measure sleep time, running 10 sleep()s in a loop and displaying the times before and after. I realized that when I tested the suggested correction I did not restore the division by 32 in the return statement. Doing that now yields correct results, and I have updated the pull request accordingly.
changed subticks calculation to give a range from 0 to 32767.
|
Yay! -my "fast" blink works now -- thank you @DavePutz |
The calculations being done in port_get_raw_ticks basically insured that only seconds were being returned. Since the tv_usec ranges from 1 to 1000000, and ticks are 1/1024 of a second, the correct calculation would be to divide usec by 1024, not 32768 and then 32. A quick script to show the issue and fix:
Before the fix the first loop will run for 10 seconds and the second loop for 20 seconds.
After the fix the timings should be correct.