LWIP: bug in sys_now Topic is solved

Report here problems in any of ChibiOS components. This forum is NOT for support.
steved
Posts: 833
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 137 times

Re: LWIP: bug in sys_now

Postby steved » Tue Aug 27, 2024 6:26 pm

Giovanni wrote:Any feedback?

Giovanni

I like that general approach:
a) It's reasonable to require systick >=1kHz (I haven't seen systick <1kHz on any of the examples, and 10kHz or more seems about right for the faster processors, which are the ones with Ethernet).
b) One function regardless - no test cases to miss.

I've pretty much convinced myself that this proposal works for systick of 5kHz, 10kHz, 50kHz, 100kHz. And nearly convinced myself that my test method is wrong! Will have another look tomorrow, hopefully.

steved
Posts: 833
Joined: Fri Nov 09, 2012 2:22 pm
Has thanked: 12 times
Been thanked: 137 times

Re: LWIP: bug in sys_now

Postby steved » Wed Aug 28, 2024 10:36 am

Go for the "Giovanni" approach. (Although a bit more testing than I've done wouldn't be a bad idea)

My suggestion definitely doesn't work with 16-bit systick (not that it was intended to). There may also be a problem with higher frequency 32-bit systicks, but I'm not inclined to pursue what was intended to be a "30-minute" fix!

User avatar
Giovanni
Site Admin
Posts: 14563
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1111 times
Been thanked: 937 times
Contact:

Re: LWIP: bug in sys_now

Postby Giovanni » Wed Aug 28, 2024 12:20 pm

Committed with a small change, incredible how such a minor thing can impact code size so much:

Code: Select all

#if (OSAL_ST_FREQUENCY < 1000) || ((OSAL_ST_FREQUENCY % 1000) != 0)
#error "LWIP requires a systick frequency that is a multiple of 1000"
#endif

#if (OSAL_ST_RESOLUTION >= 32) && (OSAL_ST_FREQUENCY == 1000)
u32_t sys_now(void) {return (u32_t)osalOsGetSystemTimeX();}

#else
u32_t sys_now(void) {
  static struct {
    systime_t   last_system_time;
    u32_t       last_ms;
    u32_t       last_unprocessed;
  } persistent = {0, 0, 0};
  u32_t delta;
  systime_t now_time;

  /* Calculating delta, in ticks, from the last acquired system time.*/
  now_time = osalOsGetSystemTimeX();
  delta = (u32_t)osalTimeDiffX(persistent.last_system_time, now_time) +
          persistent.last_unprocessed;
  persistent.last_system_time = now_time;

  /* Storing this milliseconds time and eventual remainder.*/
  persistent.last_ms          += delta / (OSAL_ST_FREQUENCY / 1000U);
  persistent.last_unprocessed  = delta % (OSAL_ST_FREQUENCY / 1000U);

  return persistent.last_ms;
}
#endif


Giovanni


Return to “Bug Reports”

Who is online

Users browsing this forum: No registered users and 8 guests