Skip to content

pkg/nordic_softdevice_ble: Use MAC48 as hardware address#10514

Merged
cgundogan merged 1 commit intoRIOT-OS:masterfrom
miri64:pkg/enh/nordic-softdevice-ble-mac48
Jan 17, 2019
Merged

pkg/nordic_softdevice_ble: Use MAC48 as hardware address#10514
cgundogan merged 1 commit intoRIOT-OS:masterfrom
miri64:pkg/enh/nordic-softdevice-ble-mac48

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Nov 29, 2018 •
edited
Loading

Contribution description

This is just a compatibility issue waiting to happen as soon as there is support for a more standard-compliant implementation of BLE (like e.g. NimBLE ;-)).

Testing procedure

gnrc_networking should still work with nordic_softdevice_ble (see README). I don't currently have the hardware at hand to test, so I pushed untested.

Issues/PRs references

#10521 and should also help with @haukepetersen's NimBLE to GNRC integration work.

gnrc_netif/gnrc_sixlowpan_iphc BLE capability

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking Area: pkg Area: External package ports Area: BLE Area: Bluetooth Low Energy support labels Nov 29, 2018
miri64 added a commit to miri64/RIOT that referenced this pull request Nov 29, 2018
@miri64
Merge branch 'pkg/enh/nordic-softdevice-ble-mac48' (RIOT-OS#10514) in…
1437a44 
…to gnrc_netif/enh/iid-conversion
@miri64 miri64 mentioned this pull request Nov 29, 2018
Merged
6 tasks
@miri64 miri64 force-pushed the pkg/enh/nordic-softdevice-ble-mac48 branch 2 times, most recently from b68e4ae to afbb774 Compare November 29, 2018 15:33
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

Rebased to current master.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

I just noticed, that this PR isn't possible without #10521, so I'll shuffle around some dependencies tomorrow =D.

@miri64 miri64 force-pushed the pkg/enh/nordic-softdevice-ble-mac48 branch 2 times, most recently from 863f1d7 to 3485467 Compare November 30, 2018 09:13
miri64 added a commit to miri64/RIOT that referenced this pull request Nov 30, 2018
@miri64
Merge branch 'gnrc_sixlowpan_iphc/fix/non-ieee802154-support' (RIOT-O…
40131a0 
…S#10514) into HEAD
@miri64 miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Nov 30, 2018
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 30, 2018

Rebased and adapted for current #10521.

@miri64 miri64 force-pushed the pkg/enh/nordic-softdevice-ble-mac48 branch from 3485467 to 0349528 Compare December 5, 2018 15:15
miri64 added a commit to miri64/RIOT that referenced this pull request Dec 5, 2018
@miri64
Merge branch 'gnrc_sixlowpan_iphc/fix/non-ieee802154-support' (RIOT-…
a1c5ed5 
…OS#10514) into HEAD
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 5, 2018

Rebased to current master and dependencies

@miri64 miri64 force-pushed the pkg/enh/nordic-softdevice-ble-mac48 branch from 0349528 to 4090be1 Compare December 9, 2018 18:56
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 9, 2018

Rebased to current master. No longer depending on any PRs

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: waiting for other PR State: The PR requires another PR to be merged first labels Dec 9, 2018
@miri64 miri64 force-pushed the pkg/enh/nordic-softdevice-ble-mac48 branch 3 times, most recently from 23e4652 to f826de1 Compare December 13, 2018 16:15
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 13, 2018

@kaspar030 do you have the hardware to test? I tested it now and it should work (@haukepetersen reported earlier offline that the softdevice did not work, but for me it worked in both this PR and in current master in communication with an Ubuntu Linux 4.4.0-140-generic #166-Ubuntu SMP Wed Nov 14 20:09:47 UTC 2018)

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Dec 17, 2018

@aabadie maybe you can test?

@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Jan 16, 2019

@PeterKietzmann and I tried to use the softdevice but unfortunately, we couldn't get the bt0 interface up and running. In fact, it didn't even show up in the interface list. Nevertheless, there was one difference with this PR compared to master, though:

Without this PR, we can connect to the RIOT device:

hcitool con 
Connections:
	< LE 00:46:49:BA:C9:50 handle 0 state 5 lm MASTER

and with this PR, the connection cannot be established, meaning, hcitool con shows an empty list.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 16, 2019

OK. I'll have a look.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 16, 2019

When on master I don't see the device using hcitool as you described:

# hcitool con
Connections:

(neither do I see it in this branch). However, in this branch I can successfully establish a connection as described in the README and ping the node.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 16, 2019 •
edited
Loading

Would have also surprised me because I did not change any behavior of the actual driver. Just how RIOT sees [edit]and handles addresses in interaction with[/edit] the soft device.

@miri64 miri64 closed this Jan 16, 2019
@miri64 miri64 reopened this Jan 16, 2019
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 16, 2019

Accidentally hit the wrong button ^^"

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 16, 2019

Ah now on master when I am connected it shows me something.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 16, 2019

But so it does on this branch.... seems to me hcitool con is a little flaky.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 17, 2019

Ok, I did some research, but I'm not sure. The nRF5 SDK documentation has a compile time switch to make its BLE implementation compatible with Linux Kernels >=4.12. This macro doesn't exist in the nRF5 IoT SDK the nordic_softdevice_ble pkg is using. So there is at least the suspicion, that the package as is doesn't work with Linux >=4.12.

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 17, 2019

Any chance to get your hands on a lesser current version of Linux to test this ^^?

cgundogan
cgundogan approved these changes Jan 17, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

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

Martine and I tested this PR on an ubuntu 16.04 with a 4.4 kernel. I can confirm that the address is shortened to 6 bytes and that pinging still works between linux <-> RIOT+softdevice.

ACK. Please squash!

@miri64 miri64 force-pushed the pkg/enh/nordic-softdevice-ble-mac48 branch from f826de1 to 85ee1d2 Compare January 17, 2019 16:15
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 17, 2019

Squashed

@cgundogan cgundogan added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines labels Jan 17, 2019
@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Jan 17, 2019

@miri64 Murdock complains about doxygen

@miri64
pkg/nordic_softdevice_ble: Use MAC48 as hardware address
7f7bc8f 
This is just a compatibility issue waiting to happen as soon as there
is support for a more standard-compliant implementation of BLE (like
e.g. NimBLE ;-)).
@miri64 miri64 force-pushed the pkg/enh/nordic-softdevice-ble-mac48 branch from 85ee1d2 to 7f7bc8f Compare January 17, 2019 17:09
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Jan 17, 2019

Fixed and directly amended.

@cgundogan cgundogan merged commit 82f30fd into RIOT-OS:master Jan 17, 2019
@miri64 miri64 deleted the pkg/enh/nordic-softdevice-ble-mac48 branch January 17, 2019 18:28
This was referenced Jan 17, 2019
Merged
Merged
Merged
@aabadie aabadie added this to the Release 2019.01 milestone Jan 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cgundogan cgundogan cgundogan approved these changes

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@kaspar030 kaspar030 Awaiting requested review from kaspar030

Assignees

No one assigned

Labels

Area: BLE Area: Bluetooth Low Energy support Area: network Area: Networking Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2019.01

Development

Successfully merging this pull request may close these issues.

3 participants

@miri64 @cgundogan @aabadie