-
Notifications
You must be signed in to change notification settings - Fork 2.1k
ieee802154: provide general header build and read functions #4636
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
Conversation
aef7958
to
0bd49a7
Compare
Maybe @A-Paul wants to look over the unittests, too. But be aware that I wrote them from the stand-point that one at least knows the make-up of a IEEE 802.15.4 frame header (little-endian, so first byte/bit in table is least significant), which should be expected if you write a driver for a network device for that protocol ;-):
where FCF is a bitfield of the following format
For the specific values of the FCF fields please see the |
1ba2c1e
to
0c3f718
Compare
Rebased and squashed |
@aeneby #4645 (comment): if you want those you need to provide them... My knowledge about IEEE802.15.4 is not sufficient enough for that :/ |
@authmillenon something like: #define IEEE802154_MIN_FREQ 2405 /**< Min. frequency (2405 MHz) */
#define IEEE802154_MAX_FREQ 2480 /**< Max. frequency (2480 MHz) */
#define IEEE802154_MIN_CHANNEL 11 /**< Min. channel (2405 MHz) */
#define IEEE802154_MAX_CHANNEL 26 /**< Max. channel (2480 MHz) */
#define IEEE802154_CHANNEL_SPACING 5 /**< Channel spacing in MHz */ And if you want conversion macros: #define IEEE802154_CHAN2FREQ(chan) ( IEEE802154_MIN_FREQ + ((chan) - IEEE802154_MIN_CHANNEL) * IEEE802154_CHANNEL_SPACING )
#define IEEE802154_FREQ2CHAN(freq) ( IEEE802154_MIN_CHANNEL + ((freq) - IEEE802154_MIN_FREQ) / IEEE802154_CHANNEL_SPACING ) |
Isn't this excluding Sub-GHz 802.15.4 radios? |
@authmillenon @aeneby 780-786 MHz (China) |
And something around 950 MHz as well |
Sorry, but that's the point where I say, that this is way to complicated for a "simple" helper macro ;-) |
Alright, I guess I'll have to read the spec regarding the other bands too. I still think it's a good idea to have these defined eventually, though. Otherwise they have to be defined per driver. |
@authmillenon said offline, that I should write: 'this should be simlified' -> so, 'this should be simplified' :-) |
Well I just quoted you ad.lib. ;-) |
5f9ca97
to
09c95c0
Compare
Reworked based on discussion at H&A with @haukepetersen (but did not adapt all unittests yet) |
09c95c0
to
0be3330
Compare
Addressed @thomaseichinger's comments in #4645 (comment) and #4645 (comment) |
Someone care to review? |
@OlegHahm you maybe since you already did some heavy reviewing in the dependent PRs? |
I didn't do any review, I just tested. |
Rebased to master and squashed. |
848ab30
to
70bbd0d
Compare
@authmillenon I'd like to review. Will go through it in the next days. |
70bbd0d
to
6a5d003
Compare
Rebased to current master. |
#define IEEE802154_FCF_DST_ADDR_SHORT (0x08) | ||
#define IEEE802154_FCF_DST_ADDR_LONG (0x0c) | ||
#define IEEE802154_FCF_DST_ADDR_VOID (0x00) /**< no destination address */ | ||
#define IEEE802154_FCF_DST_ADDR_RESV (0x04) |
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.
Do we need a constant for a reserved field? If yes, why not document it with /**< reserved */
?
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.
I think I used it in a previous iteration of the rework. Also I used it in the tests to generate the corner case "User sets wrong flags". Will add doc line.
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.
Addressed
@kaspar030, found time? |
@kaspar030 can I have a drink to celebrate tonight too? :P |
Looking through this I can't find anything to complain about, so ACK from my side (once Travis agrees of course). |
1039b72
to
ab0505d
Compare
Rebased and squashed. |
ACK from my side, too. This doesn't touch any existing code, and RIOT CI looks good, so feel free to merge! |
I thought we say we prefer still Travis for this week. |
I thought we take RIOT CI's decisions with a grain of salt and actually take a look at it's result. |
ieee802154: provide general header build and read functions
Travis is done now to, so merge ;-P |
First step in a direction to unify IEEE 802.15.4 drivers similar as Ethernet drivers are (and move them to netdev2 in the process).