kernel: reimplement k_uptime_get_32() #18744
Conversation
|
This is an alternative solution; contrast with #18742. |
Easy:
vs
Instant win. |
|
We should think if we do want to deprecate this API as well. |
include/kernel.h
Outdated
| * This routine returns the lower 32-bits of the elapsed time since the system | ||
| * booted, in milliseconds. | ||
| * This routine returns a value obtained by converting the lower | ||
| * 32-bits of the system clock into milliseconds. |
There was a problem hiding this comment.
This change looks like it got left in by mistake. In fact the effect of this patch is to make the old text true (where it wasn't before). The new text reflects the old behavior.
There was a problem hiding this comment.
Oh wait, no, the old text wasn't right either. Suggest "This routine returns the lower 32 bits of the system uptime in milliseconts". It's not the result of converting only the lower 32 bits anymore.
There was a problem hiding this comment.
Replaced with suggested text.
include/kernel.h
Outdated
| * This routine returns the lower 32-bits of the elapsed time since the system | ||
| * booted, in milliseconds. | ||
| * This routine returns a value obtained by converting the lower | ||
| * 32-bits of the system clock into milliseconds. |
There was a problem hiding this comment.
Oh wait, no, the old text wasn't right either. Suggest "This routine returns the lower 32 bits of the system uptime in milliseconts". It's not the result of converting only the lower 32 bits anymore.
The current implementation does not return the low 32 bits of k_uptime_get() as suggested by it's documentation; it returns the number of milliseconds represented by the low 32-bits of the underlying system clock. The truncation before translation results in discontinuities at every point where the system clock increments bit 33. Reimplement it using the full-precision value, and update the documentation to note that this variant has little value for long-running applications. Closes zephyrproject-rtos#18739. Signed-off-by: Peter Bigot <[email protected]>
pabigot
force-pushed
the
nordic/20190828b
branch
from
87659ab to
72b1f2f
Compare
Why would we want to deprecate it. Between k_uptime_get_32() and k_uptime_get_64, the latter is "more superfluous". It's perfectly possible to write a lot of useful software with 32-bit version (and thus remove 64-bit version, and implement optimized 32-bit). I'd like to contrast that with a case in zephyrproject-rtos/sdk-ng#101 . There, there's official standard for 64-bit support, people rely on it, and Zephyr misses it. And the comments solicited? we did not need that the last 5 years, why change now? So, we didn't need need to force 64-bit timestamps for 5 years, indeed, even "deprecation" patch #18742 just casts 64-bit value to 32-bit. So why would we deprecate 32-bit timestamps now, if they were part of the API all along? (Oh yeah, I didn't look into #17155 and don't have an idea what it may bring...). |
The current implementation does not return the low 32 bits of
k_uptime_get() as suggested by it's documentation; it returns the number
of milliseconds represented by the low 32-bits of the underlying system
clock. The truncation before translation results in discontinuities at
every point where the system clock increments bit 33.
Reimplement it using the full-precision value, and update the
documentation to note that this variant has little value for
long-running applications.
Closes #18739.
Signed-off-by: Peter Bigot [email protected]