Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/gnrc/ipv6/nib: remove direct dependency to xtimer #13666

Merged
merged 3 commits into from
Mar 26, 2020

Conversation

jue89
Copy link
Contributor

@jue89 jue89 commented Mar 20, 2020

Contribution description

While working on #13661, I realized that gnrc_ipv6_nib implies that evtimer pulls in xtimer. #13661 replaces xtimer with ztimer and, thus, the implication is wrong and gnrc_ipv6_nib cannot be built due to the missing xtimer.

This contribution extends evtimer by getters to retrieve the current system time.

Testing procedure

tests/gnrc_ipv6_nib should still pass.

Issues/PRs references

#13661

sys/include/evtimer.h Outdated Show resolved Hide resolved
sys/net/gnrc/network_layer/ipv6/nib/_nib-6ln.h Outdated Show resolved Hide resolved
@@ -327,7 +327,7 @@ void gnrc_ipv6_nib_handle_pkt(gnrc_netif_t *netif, const ipv6_hdr_t *ipv6,
void gnrc_ipv6_nib_handle_timer_event(void *ctx, uint16_t type)
{
DEBUG("nib: Handle timer event (ctx = %p, type = 0x%04x, now = %ums)\n",
ctx, type, (unsigned)xtimer_now_usec() / 1000);
ctx, type, (unsigned)evtimer_now_msec());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might result in different values. Not sure though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will differ when xtimer_now_usec() just has overflown. But I think it is okay ... it's just a debug message.

@benpicco benpicco added Area: network Area: Networking Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 23, 2020
@jue89
Copy link
Contributor Author

jue89 commented Mar 24, 2020

May I squash this PR?

@miri64 Do you think your requested changes have been addressed?

@miri64
Copy link
Member

miri64 commented Mar 24, 2020

Yes, please squash. Keep the typo fix in a separate commit, please. The rest can become one commit (or two: one introducing the new functions one for the usage).

@jue89 jue89 force-pushed the feature/evtimer_now_msec branch from 6213439 to 877e0a9 Compare March 24, 2020 12:09
@jue89 jue89 requested a review from miri64 March 26, 2020 10:44
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested so far:

  • tests/gnrc_ipv6_nib on native and samr21-xpro
  • tests/gnrc_ipv6_nib_6ln on native and samr21-xpro

I am running some manual long-term tests on native and samr21-xpro to see if NDP and 6Lo-ND still work as expected.

@jue89
Copy link
Contributor Author

jue89 commented Mar 26, 2020

I am running some manual long-term tests on native and samr21-xpro to see if NDP and 6Lo-ND still work as expected.

Thank you a lot :)

Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Behavior of timers for NS, NA, RS, RA, and their respective options is as I know it is master.

@miri64 miri64 merged commit cba1a2d into RIOT-OS:master Mar 26, 2020
@jue89 jue89 deleted the feature/evtimer_now_msec branch March 26, 2020 13:43
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants