Add pdTICKS_TO_MS macro

[Dazza0]

Description

This PR adds a simple pdTICKS_TO_MS() convenience macro to convert time in ticks to time in milliseconds.

Test Steps

Checklist:

• I have tested my changes. No regression in existing tests.
• I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[htibosch]

Hi Dazza0, this is a macro that I have missed since the beginning. pdMS_TO_TICKS() is available, the opposite is not.

In the Win32 demo, we need it to create time-stamps in logging. In fact I wrote a function ulTicksToMS(). The current clock tick is translated to a time-stamp in "sec.msec".

You used the type UBaseType_t. Normally clock-ticks are expressed in TickType_t. On most platforms, both types will translate to uint32_t. That means that the expression ( xTimeInTicks * 1000U) will overflow after 4294967 ticks, or 4294 seconds ( provided that configTICK_RATE_HZ equals 1000.
That is very quickly, just a bit more that an hour.

Mentioned ulTicksToMS() is still expressed in TickType_t, I will change it to:

static TickType_t ulTicksToMS( TickType_t uxTicks )
{
    uint64_t ullCount = ( uint64_t ) uxTicks;

    ullCount = ( ullCount * ( uint64_t ) 1000U ) / configTICK_RATE_HZ;
    return ( uint32_t ) ullCount;
}

As for the pdTICKS_TO_MS() macro, an overflow after one hour is very disturbing. I would propose to use a temporary cast to uint64_t when calculating xTimeInTicks * 1000.

 /* Converts a time in ticks to a time in milliseconds.  This macro can be
  * overridden by a macro of the same name defined in FreeRTOSConfig.h in case the
  * definition here is not suitable for your application. */
 #ifndef pdTICKS_TO_MS
-    #define pdTICKS_TO_MS( xTimeInTicks ) ( ( UBaseType_t ) ( ( ( UBaseType_t ) ( xTimeInTicks ) * ( UBaseType_t ) 1000U ) / ( UBaseType_t ) configTICK_RATE_HZ ) )
+    #define pdTICKS_TO_MS( xTimeInTicks ) ( ( TickType_t ) ( ( ( uint64_t ) ( xTimeInTicks ) * ( uint64_t ) 1000U ) / ( uint64_t ) configTICK_RATE_HZ ) )
 #endif

Thanks, Hein

[htibosch]

I just tested my proposed macro on a 32-bit platform. It works as expected, although for a complete test, you'd have to adapt pdMS_TO_TICKS() as well:

-    #define pdMS_TO_TICKS( xTimeInMs )    ( ( TickType_t ) ( ( ( TickType_t ) ( xTimeInMs )   * ( TickType_t ) configTICK_RATE_HZ ) / ( TickType_t ) 1000U ) )
+    #define pdMS_TO_TICKS( xTimeInMs )    ( ( TickType_t ) ( ( ( uint64_t ) ( xTimeInMs )   * ( uint64_t ) configTICK_RATE_HZ ) / ( uint64_t ) 1000U ) )

I tested with a tick rate of 500 Hz.

[Dazza0]

@htibosch Thanks for the quick review. I've update both pdTICKS_TO_MS() and pdMS_TO_TICKS() to use a uint64_t cast.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0640b2e) 93.51% compared to head (acc71c7) 93.51%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #866   +/-   ##
=======================================
  Coverage   93.51%   93.51%           
=======================================
  Files           6        6           
  Lines        3175     3175           
  Branches      883      883           
=======================================
  Hits         2969     2969           
  Misses         99       99           
  Partials      107      107           

Flag	Coverage Δ
unittests	93.51% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[aggarg]

Given that we now have a way to make TickType_t 64-bit, do we need this to be explicitly uint64_t?

I think that we do not and TickType_t should be good. Those who need wider TickType_t can do so in their FreeRTOSConfig.h. What do you think @Dazza0, @htibosch ?

[htibosch]

The problem is that it may take a lot of time before one discovers that mentioned macro is overflowing. I am one of them: I couldn't understand why the timestamps in my logging were erroneous.

And I can imagine that someone choses TICK_TYPE_WIDTH_32_BITS and conversion macro's that do not overflow in 1.5 day.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication