lib: date_time: use millisecond precision in ntp api

[syscrypt]

Hi,
the ntp part of the date_time lib ignores the fraction field returned by the ntp protocol, which leads to an accuracy within one second. I am aware that the date_time api docs mention that the fetched time is a UTC date time stamp so I guess this is expected behavior. But it is a little bit confusing that the api date_time_now method expects an int64_t pointer to a var called unix_time_ms which then has an accuracy of a second.

Is it possible to update the ntp part to provide a millisecond accuracy as in my pr?

Regards

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[NordicBuilder]

Thank you for your contribution!
It seems you are not a member of the nrfconnect GitHub organization. External contributions are handled as follows:
Large contributions, affecting multiple subsystems for example, may be rejected if they are complex, may introduce regressions due to lack of test coverage, or if they are not consistent with the architecture of nRF Connect SDK.
PRs will be run in our continuous integration (CI) test system.
If CI passes, PRs will be tagged for review and merged on successful completion of review. You may be asked to make some modifications to your contribution during review.
If CI fails, PRs may be rejected or may be tagged for review and rework.
PRs that become outdated due to other changes in the repository may be rejected or rework requested.
External contributions will be prioritized for review based on the relevance to current development efforts in nRF Connect SDK. Bug fix PRs will be prioritized.
You may raise issues or ask for help from our Technical Support team by visiting https://devzone.nordicsemi.com/.

Note: This comment is automatically posted and updated by the Contribs GitHub Action.

[tokangas]

Thanks for your contribution, @syscrypt. The implementation seems to have been like this already in the first version. I don't know the reason, but I don't see why we should ignore the fraction.

Your change looks good to me, however you need to sign the CLA and the commit description needs some fine tuning to pass the compliance checks.

The commit message needs to have a body and a "Signed-off-by" line. We follow the same contribution guidelines as Zephyr, here are instructions for the commit message: https://developer.nordicsemi.com/nRF_Connect_SDK/doc/latest/zephyr/contribute/guidelines.html#commit-message-guidelines

[rlubos]

Please follow the commit message guidelines (https://docs.zephyrproject.org/latest/contribute/guidelines.html#commit-message-guidelines), in short:

• The commit message needs some change description in the body
• it needs a signoff entry

[NordicBuilder]

Test specification

CI/Jenkins/NRF

• Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
test-fw-nrfconnect-nrf-iot_mosh	X
test-fw-nrfconnect-nrf-iot_positioning	X
test-fw-nrfconnect-nrf-iot_serial_lte_modem	X
test-fw-nrfconnect-nrf-iot_thingy91	X
test-fw-nrfconnect-nrf-iot_zephyr_lwm2m	X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

[syscrypt]

Thanks for the quick turnaround, I have adjusted the commit.

[trantanen]

But it is a little bit confusing that the api date_time_now method expects an int64_t pointer to a var called unix_time_ms which then has an accuracy of a second.

Zephyr has a convention of using time as int64_t in milliseconds so regardless of what would make sense in a particular API, this convention is still followed in many places, such as Location library API.

Hi, the ntp part of the date_time lib ignores the fraction field returned by the ntp protocol, which leads to an accuracy within one second.

While this change makes sense and is fine for me, we will always have an accuracy issue because there is data transfer delay in returning the NTP time from the server and then getting it through the device's SW architecture. So this change removes some inaccuracy but there is still a random delay which can be seconds or even tens of seconds in some cases depending on cellular network conditions.

[trantanen]

I think a comment would be nice to say that the fraction part is not milliseconds, microseconds or nanoseconds but rather fractions of a second based on 32 bit unsigned integer maximum value.
I know that's what the code does or "says" but given this is very weird unit, it would be good to have a comment alarming the reader about it.

[tokangas]

@syscrypt You still need to sign the CLA, see #11847 (comment).

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.