Skip to content

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

Merged
miri64 merged 3 commits intoRIOT-OS:masterfrom
jue89:feature/evtimer_now_msec
Mar 26, 2020
Merged

net/gnrc/ipv6/nib: remove direct dependency to xtimer#13666
miri64 merged 3 commits intoRIOT-OS:masterfrom
jue89:feature/evtimer_now_msec

Conversation

@jue89
Copy link
Copy Markdown
Contributor

@jue89 jue89 commented Mar 20, 2020 •
edited
Loading

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

This was referenced Mar 20, 2020
Merged
Open
miri64
miri64 reviewed Mar 20, 2020
View reviewed changes
miri64
miri64 requested changes Mar 20, 2020
View reviewed changes
sys/net/gnrc/network_layer/ipv6/nib/nib.c
{
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
Copy Markdown
Member

@miri64 miri64 Mar 20, 2020

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
Copy Markdown
Contributor Author

@jue89 jue89 Mar 20, 2020

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
Copy Markdown
Contributor Author

jue89 commented Mar 24, 2020 •
edited
Loading

May I squash this PR?

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

@miri64
Copy link
Copy Markdown
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
miri64
miri64 reviewed Mar 26, 2020
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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 :)

@miri64 miri64 mentioned this pull request Mar 26, 2020
Merged
miri64
miri64 approved these changes Mar 26, 2020
View reviewed changes
Copy link
Copy Markdown
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

Reviewers

@miri64 miri64 miri64 approved these changes

@cgundogan cgundogan Awaiting requested review from cgundogan

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

Assignees

No one assigned

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

Milestone

Release 2020.04

Development

Successfully merging this pull request may close these issues.

4 participants

@jue89 @miri64 @benpicco @leandrolanzieri