auto_init: Initialize link-layer addresses from CPU ID by miri64 · Pull Request #837 · RIOT-OS/RIOT

miri64 · 2014-03-03T10:27:26Z

Maybe not the neatest idea, but as long as it stirs up some discussion it's fine.

Depends on #854

LudwigKnuepfer · 2014-03-03T10:35:02Z

sys/auto_init/auto_init.c

I don't think this should go into auto_init itself.

That was what I meant with "not the neatest idea". ;-)

LudwigKnuepfer · 2014-03-03T11:26:15Z

Can you describe which layers addresses should be initialized and what the intended use is?

kaspar030 · 2014-03-03T12:16:55Z

(PR missing tag)

miri64 · 2014-03-04T13:44:24Z

@LudwigOrtmann updated.

LudwigKnuepfer · 2014-03-04T14:11:50Z

OK, this is much better. Are there any formal requirements for the seed?

miri64 · 2014-03-04T14:18:31Z

Which seed?

LudwigKnuepfer · 2014-03-04T14:22:30Z

The "seed" used to initialize the address - i.e. the second argument to net_if_set_eui64.

miri64 · 2014-03-04T14:32:22Z

It needs to be at least 8 byte long.

miri64 · 2014-03-06T17:18:22Z

Rebased to new version of #854

LudwigKnuepfer · 2014-05-13T08:51:34Z

@authmillenon needs rebase

miri64 · 2014-05-13T13:43:09Z

Rebased to authmillenon:cpu-id

OlegHahm · 2014-06-03T12:56:43Z

Needs rebase again.

OlegHahm · 2014-06-22T17:24:30Z

@authmillenon, ping.

miri64 · 2014-06-23T07:13:29Z

Done

OlegHahm · 2014-06-24T09:47:24Z

core/include/kernel.h

Why char? Wouldn't at fixed length data type make more sense? Why not simply typedef uint16_t cpu_id_t?

Yes, but true anyway. ;)

The cpu_id_t can be longer (64 bit for iot-lab_M3 e.g.) CPU_ID_LEN == 2U is just the default, if the ~~board~~ cpu does not define anything.

(But I will do s/char/uint8_t/, there you were right.)

miri64 · 2014-06-24T12:19:04Z

Addressed comments

miri64 · 2014-07-31T21:31:21Z

Rebased to master

LudwigKnuepfer · 2014-08-01T06:41:44Z

sys/auto_init/auto_init.c

I think the time has come to refactor this part of auto_init into a dedicated function. (In a prior commit.)

which part exactly?

I'd opt for the complete net_if code.

LudwigKnuepfer · 2014-08-01T06:45:04Z

It's kaput =(

tests/coap:

/home/lo/native/RIOT/sys/auto_init/auto_init.c: In function ‘auto_init’:
/home/lo/native/RIOT/sys/auto_init/auto_init.c:151:9: warning: implicit declaration of function ‘cpuid_get’ [-Wimplicit-function-declaration]
         cpuid_get(cpuid);

LudwigKnuepfer · 2014-08-09T11:58:19Z

sys/auto_init/auto_init.c

why printf here?

Because it's inside a DEBUG_ENABLED ifdef anyways

but there's DEBUG in there as well..

LudwigKnuepfer · 2014-08-09T12:02:17Z

I'm not sure if it would be cleaner to move the init function into its own file in the net_if module.. I think it would look cleaner and one could use it without also using auto_init.

miri64 · 2014-08-09T12:23:52Z

I think not. All this function does is initializing the module net_if_init(); (what is also the case for all other modules in auto_init) and setting some values a user normally does not want to be bothered with (channel, radio address, long address and PAN ID). It just goes through the cases, if a CPU ID is given or not. This is auto-initialization, it's just that a network interface has a whole lot more to do, than a "normal" module. All in all, if you want the functionality of this function, you normally want auto_init, but if you do not want auto_init, then you normally do not want the functionality of this function. Optimally these values are given by the hardware itself, but apparently they are not.

LudwigKnuepfer · 2014-08-09T12:26:12Z

What about the separation of concerns (cleanliness)?

miri64 · 2014-08-09T12:35:23Z

This functionality will look ugly in either way. As far as I'm concerned this initialization shouldn't even be net_if's task, but the of the network device driver since they obviously know best, what there addresses, channels etc look like.

LudwigKnuepfer · 2014-08-09T12:36:51Z

OK

LudwigKnuepfer · 2014-08-27T17:48:36Z

Is this ready from your point of view?

miri64 · 2014-08-28T10:37:06Z

Yes

LudwigKnuepfer · 2014-08-28T10:39:02Z

OK, I'll restart Travis for master compatibility checking.

OlegHahm · 2014-09-12T15:12:43Z

Restarted again.

OlegHahm · 2014-09-17T21:14:03Z

Needs blacklisting.

OlegHahm · 2014-09-30T12:02:41Z

@authmillenon, can you add the necessary blacklistings? I think otherwise this should be ready to merge.

miri64 · 2014-09-30T12:07:58Z

This is not an application. How/what should I blacklist?

OlegHahm · 2014-09-30T12:21:26Z

The failing applications: https://travis-ci.org/RIOT-OS/RIOT/builds/32056162

miri64 · 2014-09-30T12:31:11Z

I think I better check why next to every application is not compiling for Arduino Mega, before I remove support for this board ;-)

miri64 · 2014-09-30T12:33:54Z

Rebased => There I fixed it ;-)

OlegHahm · 2014-09-30T12:35:07Z

sys/auto_init/auto_init.c

Moved to https://github.com/authmillenon/RIOT/blob/auto-init-addresses/sys/auto_init/auto_init.c#L228

Ah, missed that.

OlegHahm · 2014-09-30T12:37:55Z

If Travis is fine, I am fine, too. You can call this an ACK.

miri64 · 2014-09-30T12:47:20Z

Kicked travis

OlegHahm · 2014-09-30T12:59:40Z

and again.

OlegHahm · 2014-09-30T13:29:46Z

\o/ - and go.

auto_init: Initialize link-layer addresses from CPU ID

miri64 · 2014-09-30T15:40:52Z

Finally

LudwigKnuepfer reviewed Mar 3, 2014
View reviewed changes

LudwigKnuepfer added the enhancement label Mar 3, 2014

miri64 mentioned this pull request Mar 4, 2014

drivers/cpu: add function to get CPU id/serial number #854

Merged

OlegHahm added the network label May 16, 2014

OlegHahm added this to the Release NEXT MAJOR milestone May 16, 2014

OlegHahm assigned emmanuelsearch May 16, 2014

OlegHahm modified the milestones: FIX ME FIRST, Release NEXT MAJOR Jun 3, 2014

OlegHahm reviewed Jun 24, 2014
View reviewed changes

LudwigKnuepfer reviewed Aug 1, 2014
View reviewed changes

LudwigKnuepfer reviewed Aug 9, 2014
View reviewed changes

miri64 mentioned this pull request Aug 9, 2014

make: add tests/Makefile that runs all tests #1556

Closed

Martin Lenders and others added 2 commits September 30, 2014 14:32

Initialize addresses from CPU ID

30c92ef

auto_init: Put net_if auto-initialization into its own function

a5a7008

miri64 force-pushed the auto-init-addresses branch from e8fa71f to a5a7008 Compare September 30, 2014 12:32

OlegHahm reviewed Sep 30, 2014
View reviewed changes

OlegHahm assigned OlegHahm and unassigned emmanuelsearch Sep 30, 2014

OlegHahm added a commit that referenced this pull request Sep 30, 2014

Merge pull request #837 from authmillenon/auto-init-addresses

be1fde1

auto_init: Initialize link-layer addresses from CPU ID

OlegHahm merged commit be1fde1 into RIOT-OS:master Sep 30, 2014

miri64 deleted the auto-init-addresses branch October 1, 2014 09:42

Conversation

miri64 commented Mar 3, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LudwigKnuepfer commented Mar 3, 2014

Uh oh!

kaspar030 commented Mar 3, 2014

Uh oh!

miri64 commented Mar 4, 2014

Uh oh!

LudwigKnuepfer commented Mar 4, 2014

Uh oh!

miri64 commented Mar 4, 2014

Uh oh!

LudwigKnuepfer commented Mar 4, 2014

Uh oh!

miri64 commented Mar 4, 2014

Uh oh!

miri64 commented Mar 6, 2014

Uh oh!

LudwigKnuepfer commented May 13, 2014

Uh oh!

miri64 commented May 13, 2014

Uh oh!

OlegHahm commented Jun 3, 2014

Uh oh!

OlegHahm commented Jun 22, 2014

Uh oh!

miri64 commented Jun 23, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miri64 commented Jun 24, 2014

Uh oh!

miri64 commented Jul 31, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LudwigKnuepfer commented Aug 1, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LudwigKnuepfer commented Aug 9, 2014

Uh oh!

miri64 commented Aug 9, 2014

Uh oh!

LudwigKnuepfer commented Aug 9, 2014

Uh oh!

miri64 commented Aug 9, 2014

Uh oh!

LudwigKnuepfer commented Aug 9, 2014

Uh oh!

LudwigKnuepfer commented Aug 27, 2014

Uh oh!

miri64 commented Aug 28, 2014

Uh oh!

LudwigKnuepfer commented Aug 28, 2014

Uh oh!

OlegHahm commented Sep 12, 2014

Uh oh!

OlegHahm commented Sep 17, 2014