-
Notifications
You must be signed in to change notification settings - Fork 2k
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
gnrc_mac: add timeout module. #5949
gnrc_mac: add timeout module. #5949
Conversation
2896be9
to
a0a4550
Compare
@zhuoshuguo I have some ideas how to integrate this more concise with the rest of GNRC, but did not find any time to realize them yet. Would you mind to meet up or something like that next week so we can merge our ideas? |
@miri64 Definitely, so, let's meet up next week, let's say Monday, maybe through Skype, what do you think? |
@miri64 Hi, when do you think we can talk about this? :-) My plan is to merge the related MAC modules and finally the Lw-MAC before the recent RIOT release, i.e., the end of October. Do you think it is possible? |
gnrc_netdev2_t* gnrc_netdev2; | ||
|
||
/* Store timeouts used for protocol */ | ||
gnrc_mac_timeout_t timeouts[GNRC_MAC_TIMEOUT_COUNT]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This too can just be added directly to gnrc_netdev2
with zhuoshuguo#2
Lot of what I see here is related to what I plan for NDP (a timer-based queue). @kaspar030 could you maybe finalize some version of your event timer? Maybe considering the functionalities @zhuoshuguo is requiring here, too? |
I'm sorry, there still needs to be put a lot of clean-up work into these PRs (and consolidation with existing types or types that already planned by other people, see above) so I don't think we manage to include these (and thus Lw-MAC as well) into this release :-(. But let's see, where we stand next week. |
@miri64 Thanks, Martine, I see that there are indeed lots of clean-up work waiting to be done, which is also far more than what I expect @emmanuelsearch @OlegHahm (actually, I am little in mess now) . Thus, I understand that it could be difficult to include those PRs (and Lw-MAC) into this release. Anyway, I will continue to digest your suggestions for me and my PRs, and figure out how to adjust Lw-MAC for those newly coming features. Talk to you tomorrow. :-) |
@zhuoshuguo @kaspar030 published his work at #5999 by now. Could you maybe have a look at it and see how you can adapt? If you have any questions, just ask. |
@miri64 Thanks, Martine, I will check how I can adapt. |
a0a4550
to
7881cbe
Compare
Needs rebase. |
337b6ac
to
23dec57
Compare
Rebased. |
@zhuoshuguo please only squash when asked to. Also: you squashed #5999 into your commit now too. |
Opps...my fault. Anyway, I think this PR will be only merged after #5999, so then, I can still adjust, right? |
c5f6575
to
16c2275
Compare
Updated, removed evtimer from original commit. Added descriptions. |
16c2275
to
da3bdb4
Compare
@miri64 do you think the folder which contains |
6a2a006
to
36ef66d
Compare
Rebased on master. |
Ping @zhuoshuguo? |
99f3df4
to
a9beec4
Compare
@miri64 Updated, and locally tested through on samr21-xpro. |
PS: the error reported earlier was from old version code block (guess the error missed the check of old compiler), has been adapted in the newest commit. |
tested on native and PhyNode looks good to me, though I don't know where it is actually used. But to me it looks mergeable: @miri64? I would say we don't block this for minor reasons any longer. |
Uncrustify does not report anything of interest. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests lack some documentation. Please provide at least a README and ideally a test script.
@miri64 No problem, will add some documentation soon. |
CI is not happy, some simple stuff, i.e. Arduino do not have enough mem. And then some llvm issue that might be a bit uglier? |
@miri64 Added documentation for the test example. By the way, isn't this the test script? |
@smlng any tips about how to solve these simple stuffs? :-) |
Thanks for the documentation! You've linked the main.c of your test application. That's not a script that's the program ;-). We usually provide some scripts (e.g. in python) with the test application to allow for automated execution (for some reference see the other tests in the |
Add the boards reported to the BOARD_INSUFFICIENT_MEMORY := arduino-duemilanove arduino-mega2560 arduino-uno \
chronos nucleo-f030r8 nucleo-f031k6 nucleo-f042k6 \
nucleo-l031k6 nucleo-l053r8 stm32f0discovery waspmote-pro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please squash.
Co-Authored-By: zhuoshuguo <[email protected]>
@miri64 should I also rebase? |
Not necessary (+ it makes it easier to check if there is a difference). But if it is easier for you to just rebase, do it. |
8ddccb8
to
79e5586
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK. Squash was successful.
Thanks!~ @miri64 All checks pass now. |
Add timeout module to "gnrc_mac" to provide timeout setting and handling for MAC protocols which are developed based on "gnrc_mac".
The timeout module here is built on (a wrapper of) evtimer.
Test example (
tests/gnrc_mac_timeout
in this PR) results on samr21-xrpo: