Skip to content

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jan 14, 2016

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).

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. labels Jan 14, 2016
@miri64 miri64 added this to the Release 2016.03 milestone Jan 14, 2016
@miri64 miri64 force-pushed the ieee802154/enh/general-hdr branch 4 times, most recently from aef7958 to 0bd49a7 Compare January 14, 2016 16:07
@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 14, 2016
@miri64
Copy link
Member Author

miri64 commented Jan 14, 2016

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 ;-):

Octets: 2 1 0 or 2 0, 2, or 8 0 or 2 0, 2, or 8
FCF Seq dst_pan dst_addr src_pan src_addr

where FCF is a bitfield of the following format

Bits: 0-2 3 4 5 6 7-9 10-11 12-13 14-15
Frame type Security Enabled Frame Pending ACK REQ PAN comp Reserved dst_mode version src_mode

For the specific values of the FCF fields please see the ieee802154.h

@miri64 miri64 added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Jan 24, 2016
@miri64 miri64 force-pushed the ieee802154/enh/general-hdr branch from 1ba2c1e to 0c3f718 Compare January 24, 2016 22:54
@miri64
Copy link
Member Author

miri64 commented Jan 24, 2016

Rebased and squashed

@miri64
Copy link
Member Author

miri64 commented Jan 25, 2016

@aeneby #4645 (comment): if you want those you need to provide them... My knowledge about IEEE802.15.4 is not sufficient enough for that :/

@aeneby
Copy link
Member

aeneby commented Jan 25, 2016

@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 )

@miri64
Copy link
Member Author

miri64 commented Jan 25, 2016

Isn't this excluding Sub-GHz 802.15.4 radios?

@jnohlgard
Copy link
Member

@authmillenon @aeneby
These are some other bands which we need to support as well:

780-786 MHz (China)
868.3 MHz (EU)
906-924 MHz (North America)
Ultra wide band:
249.6 MHz to 749.6 MHz
3.1 GHz to 4.8 GHz
6.0 GHz to 10.6 GHz

@jnohlgard
Copy link
Member

And something around 950 MHz as well

@miri64
Copy link
Member Author

miri64 commented Jan 25, 2016

Sorry, but that's the point where I say, that this is way to complicated for a "simple" helper macro ;-)

@aeneby
Copy link
Member

aeneby commented Jan 25, 2016

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.

@haukepetersen
Copy link
Contributor

@authmillenon said offline, that I should write: 'this should be simlified' -> so, 'this should be simplified' :-)

@miri64
Copy link
Member Author

miri64 commented Jan 26, 2016

Well I just quoted you ad.lib. ;-)

@miri64 miri64 removed the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Jan 26, 2016
@miri64 miri64 force-pushed the ieee802154/enh/general-hdr branch 2 times, most recently from 5f9ca97 to 09c95c0 Compare January 27, 2016 02:46
@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 27, 2016
@miri64
Copy link
Member Author

miri64 commented Jan 27, 2016

Reworked based on discussion at H&A with @haukepetersen (but did not adapt all unittests yet)

@miri64 miri64 force-pushed the ieee802154/enh/general-hdr branch from 09c95c0 to 0be3330 Compare January 27, 2016 13:56
@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 1, 2016
@miri64
Copy link
Member Author

miri64 commented Feb 1, 2016

Addressed @thomaseichinger's comments in #4645 (comment) and #4645 (comment)

@miri64
Copy link
Member Author

miri64 commented Feb 12, 2016

Someone care to review?

@miri64
Copy link
Member Author

miri64 commented Feb 16, 2016

@OlegHahm you maybe since you already did some heavy reviewing in the dependent PRs?

@OlegHahm
Copy link
Member

I didn't do any review, I just tested.

@miri64
Copy link
Member Author

miri64 commented Feb 23, 2016

Rebased to master and squashed.

@miri64 miri64 force-pushed the ieee802154/enh/general-hdr branch from 848ab30 to 70bbd0d Compare February 23, 2016 12:25
@kaspar030
Copy link
Contributor

@authmillenon I'd like to review. Will go through it in the next days.

@kaspar030 kaspar030 assigned kaspar030 and unassigned haukepetersen Feb 23, 2016
@miri64 miri64 force-pushed the ieee802154/enh/general-hdr branch from 70bbd0d to 6a5d003 Compare February 24, 2016 12:56
@miri64
Copy link
Member Author

miri64 commented Feb 24, 2016

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)
Copy link
Member

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 */?

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed

@OlegHahm OlegHahm added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Feb 27, 2016
@OlegHahm
Copy link
Member

@authmillenon I'd like to review. Will go through it in the next days.

@kaspar030, found time?

@miri64
Copy link
Member Author

miri64 commented Mar 6, 2016

@kaspar030 can I have a drink to celebrate tonight too? :P

@haukepetersen haukepetersen added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 7, 2016
@haukepetersen
Copy link
Contributor

Looking through this I can't find anything to complain about, so ACK from my side (once Travis agrees of course).

@miri64 miri64 force-pushed the ieee802154/enh/general-hdr branch from 1039b72 to ab0505d Compare March 7, 2016 15:38
@miri64
Copy link
Member Author

miri64 commented Mar 7, 2016

Rebased and squashed.

@kaspar030
Copy link
Contributor

ACK from my side, too. This doesn't touch any existing code, and RIOT CI looks good, so feel free to merge!

@miri64
Copy link
Member Author

miri64 commented Mar 7, 2016

I thought we say we prefer still Travis for this week.

@kaspar030
Copy link
Contributor

I thought we take RIOT CI's decisions with a grain of salt and actually take a look at it's result.
This one looks fine. Your call.

miri64 added a commit that referenced this pull request Mar 7, 2016
ieee802154: provide general header build and read functions
@miri64 miri64 merged commit 32e0c7a into RIOT-OS:master Mar 7, 2016
@miri64
Copy link
Member Author

miri64 commented Mar 7, 2016

Travis is done now to, so merge ;-P

@miri64 miri64 deleted the ieee802154/enh/general-hdr branch March 7, 2016 20:03
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 Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. 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.

6 participants