Continuous time update - Alternative implementation to #2041

[JF002]

This is an alternative implementation to #2041 we talked about in this comment (https://github.com/InfiniTimeOrg/InfiniTime/pull/2041#issuecomment-2081533165).

This implementation does not change the state of the DateTime controller (previousSystickCounter and currentDateTime fields) in GetCurrentDateTime(). This allows to keep the method GetCurrentDateTime() const.

I also applied a small refactoring of the methods UpdateTime() to avoid trying to lock the same mutex multiple times (FreeRTOS mutexes are not reentrant).

To test it, I slowed down SystemTask so that it runs every 5s and DisplayApp every 1s. In both tasks, I logged the return of GetCurrentDateTime().

On the original implementation, the result was:

<info> app: SYSTEMTASK : 2000
<info> app: SYSTEMTASK : 2000
<info> app: SYSTEMTASK : 3000
<info> app: DISPLAYTASK : 3000
<info> app: SYSTEMTASK : 4000
<info> app: DISPLAYTASK : 4000
<info> app: DISPLAYTASK : 4000
<info> app: DISPLAYTASK : 4000
<info> app: SYSTEMTASK : 6000
<info> app: DISPLAYTASK : 6000
<info> app: DISPLAYTASK : 6000
<info> app: DISPLAYTASK : 6000

DisplayTAsk didn't get any new value until SystemTask updated the current time.

With this implementation:

<info> app: SYSTEMTASK : 39000
<info> app: SYSTEMTASK : 39000
<info> app: DISPLAYTASK : 40000
<info> app: DISPLAYTASK : 41000
<info> app: SYSTEMTASK : 41000
<info> app: DISPLAYTASK : 41000
<info> app: SYSTEMTASK : 42000
<info> app: DISPLAYTASK : 42000
<info> app: SYSTEMTASK : 43000
<info> app: DISPLAYTASK : 43000
<info> app: SYSTEMTASK : 43000
<info> app: DISPLAYTASK : 43000
<info> app: SYSTEMTASK : 43000
<info> app: DISPLAYTASK : 43000
<info> app: DISPLAYTASK : 44000
<info> app: SYSTEMTASK : 45000

As you can see, DisplayTask gets intermediate results between 2 updates from SystemTask.

@mark9064 What do you think of this?

[github-actions[bot]]

Build size and comparison to main:	Section	Size	Difference
text	377304B	304B
data	940B	0B
bss	63540B	0B

[JF002]

@mark9064 Thank yo so much for this review!

When reading your comment, it occurred to me that redesigning the whole DateTime class should be out of scope of this PR (and the one related to the Always On display functionality). I don't want to keep your changes and work as hostage because of some limitations in the current design.

So here's my suggestion : Add a TODO file to your PR that details what need to be done to improve the design (context, analysis, objectives of the refactoring). This approach allows us to continue working on the always on feature while maintaining the technical debt under control.

Here is what this TODO file might look like:

---

Refactoring needed

Context

The PR #2041 - Continuous time update highlighted some limitations in the design of DateTimeController: the granularity of the time returned by DateTime::CurrentDateTime() is limited by the frequency at which SystemTask calls DateTime::UpdateTime(), which is currently set to 100ms.

@mark9064 provided more details in this comment.

The PR #2041 - Continuous time update provided some changes to DateTime controller that improves the granularity of the time returned by DateTime::CurrentDateTime().

However, the review showed that DateTime cannot be const anymore, even when it's only used to get the current time, since DateTime::CurrentDateTime() changes the internal state of the instance.

We tried to identify alternative implementation that would have maintained the "const correctness" but we eventually figured that this would lead to a re-design of DateTime which was out of scope of the initial PR (Continuous time updates and always on display).

So we decided to merge this PR #2041 and agree to fix/improve DateTime later on.

What needs to be done?

Improve/redesign DateTime so that it

• provides a very granular (ideally down to the millisecond) date and time via CurrentDateTime().
• can be declared/passed as const when it's only used to get the time.
• limits the use of mutex as much as possible (an ideal implement would not use any mutex, but this might not be possible).
• improves the design of DateTime::Seconds(), DateTime::Minutes(), DateTime::Hours(), etc as explained in this comment.

Once this redesign is implemented, all instances/references to DateTime should be reviewed and updated to use const where appropriate.

Please check the following PR to get more context about this redesign:

• #2041 - Continuous tie updates by @mark9064
• #2054 - Continuous time update - Alternative implementation to #2041 by @JF002

[JF002]

Closing in favor of https://github.com/InfiniTimeOrg/InfiniTime/pull/2041.