Bluetooth: Mesh: Add some actual behavior in sample #33476
Conversation
trond-snekvik
requested review from
jhedberg,
joerchan,
nashif and
Vudentz
as code owners
| #include "board.h" | ||
|
|
||
| static uint32_t oob_number; | ||
| static const struct device *gpio; | ||
| static struct mb_image onoff[] = { |
There was a problem hiding this comment.
I realize this is partially a copy-paste, but I think this can be put into ROM by making it const
trond-snekvik
force-pushed
the
mesh_sample
branch
from
437093d to
abb6e87
Compare
There was a problem hiding this comment.
Please incorporate the "fixup" patch from https://github.com/pabigot/zephyr/commits/pr/33476 into this, so it works on a wider range of boards. This is the blocking change requested.
As a comment: This doesn't quite complete the documented plan #31031 (comment) which I failed to read closely enough: I'd hoped for instructions on how to do provisioning using Zephyr, rather than an unnamed and undocumented application.
samples/bluetooth/mesh_provisioner does find and provision a device with this sample, but pressing the button produces:
The Generic OnOff Client must be bound to a key before sending.
Also there are a lot of diagnostics, including this warning:
00:02:31.743,804] <wrn> bt_mesh_health_srv: No callback for getting faults
So this still isn't as helpful as one might want in getting new users up to speed with how to use Bluetooth Mesh in Zephyr.
Perhaps the provisioner app could be extended in concert with this to provide a self-contained Zephyr sample.
trond-snekvik
force-pushed
the
mesh_sample
branch
from
abb6e87 to
c079412
Compare
|
Thanks for providing the right DeviceTree macros, I have incorporated them into the PR. |
|
@trond-snekvik Thanks for the updates. This still isn't going well. I combined #33614 with this in a development branch https://github.com/pabigot/zephyr/commits/nordic/mesh and installed the provisioner on one node, then the mesh sample on another, making sure to add The provisioner provided: Note the unusual UUID. The mesh sample produced the output below, showing successful provision then dying. Show node provisioning detailsAfter resetting the mesh node I get the following. What happens is that it doesn't seem to notice that it's provisioned right away. When I press the button to switch the LED on, it self-provisions. Show node restart detailsIf I install the mesh sample on another device I get this: Show second node provisioning detailsThis device also uses the bizarre UUID, so doesn't get provisioned. |
pabigot
dismissed
their stale review
Changes integrated so no changes requested at this time, but sample still doesn't work as expected.
There's really nothing particularly unusual about it. Many mesh samples have used that from the beginning, since it made it easy to provision them using available Linux mesh tools (instead of copy-pasting or typing the entire UUID you just typed "prov dddd" in the Linux tool. |
OK, but a hard-coded constant UUID in an auto-provisioning solution is not useful, because a mesh with one node is not useful. A UUID based on the BT identity (assuming it's the one that comes from FICR, not some random thing that goes away when the flash is erased) would make the sample usable. |
@pabigot agreed. I just wanted to stress that the hardcoded value in itself doesn’t indicate a bug somewhere, in case that’s what you were suspecting. |
|
There appears to be some instabilities in the provisioner sample, primarily caused by preemptive operation of the Bluetooth APIs. I have pushed two commits in #33614 to address this and the loopback warning. With these changes, I'm unable to reproduce the errors you have listed in the logs. I have left the UUID as-is, as I agree with @jhedberg that this hardcoding is useful. It also does not affect the provisioner sample in any meaningful way. |
Agreed, it doesn't affect the provisioner sample. It does mean the mesh sample using the Zephyr provisioner will work only for a mesh of size one. I don't understand why that would be a good thing. So https://github.com/pabigot/zephyr/commits/nordic/mesh now has a patch to fix this. Fixing the UUID I still have problems. I can get two nodes to provision, but when a third is added I get: That may be due to Also the two nodes that do get provisioned have abysmal performance: pressing the button on one causes the other's LED to sometimes change immediately, but more often change one or two seconds later; then one or two seconds after that LED on the node where the button was pushed changes. That second problem may be partially influenced by to But after changing the configs above to stop blasting out log messages and to possibly support 16 nodes, this sample is still far from providing a satisfactory out-of-box experience. Back in spring 2019 I had a Zephyr mesh sample in C++ that I managed to make work, and it was far more responsive. After several attempts at wiping out state and starting over, I still can't get a reliable 3-node mesh running where button presses are visible across the mesh in less than one second. |
|
Part of the confusion is because of the logic in |
trond-snekvik
force-pushed
the
mesh_sample
branch
from
c079412 to
91e0e48
Compare
|
I have added the hwinfo based UUID by default. It can be disabled in prj.conf if we need to run with the static UUID. This is what we're doing in Nordic's SDK as well, and although it's a bit more cumbersome, it's causing any disruptive behavior, like the dddd UUID does with the provisioner sample. I have turned off all core logging in favor of foundation model logging, which I find is the most useful when debugging. It's also not very verbose. I have also altered the button logic to invert its own LED state, good suggestion.
This surprises me, as I have not been able to reproduce on my devices at all. Is this just because of the button issue, or are you still seeing this after the latest push? Does your USB changes affect this in any way? This sample now runs purely in the work handler and BT event handler thread after the initialization - is the USB module interfering with these threads? |
samples/bluetooth/mesh/src/main.c
Outdated
| @@ -139,7 +280,7 @@ static void prov_reset(void) | |||
| bt_mesh_prov_enable(BT_MESH_PROV_ADV | BT_MESH_PROV_GATT); | |||
| } | |||
|
|
|||
| static const uint8_t dev_uuid[16] = { 0xdd, 0xdd }; | |||
| static uint8_t dev_uuid[16] = { 0xdd, 0xdd }; | |||
There was a problem hiding this comment.
FYI this could waste flash relative to the runtime assignment I suggested, since the 16-byte initial value has to be stored in flash then copied into this address during startup.
There was a problem hiding this comment.
Actually, the instructions required to do this assignment at runtime adds up to exactly 16 bytes of flash on my setup 😄 There might be small variations on other platforms and compiler versions, but I think this falls into the personal preference category
| static uint8_t app_key[16]; | ||
| int err; | ||
|
|
||
| printk("Self-provisioning with address 0x%04x\n", addr); |
There was a problem hiding this comment.
Why not use the low two bytes of the device id, so it's consistent with what a provisioner would use?
There was a problem hiding this comment.
Good point, I'll change this
I could easily reproduce when I moved the devices away from each other. Turned out to be another hidden instance of #24416 and is resolved (for this case) by #33740. At this point it appears things are working, and I'm getting decent range. Thanks. |
Populates the onoff server stubs in the Bluetooth Mesh sample, and implements generic LED based hardware support to include more boards. Fixes zephyrproject-rtos#31031. Signed-off-by: Trond Einar Snekvik <[email protected]>
trond-snekvik
force-pushed
the
mesh_sample
branch
from
91e0e48 to
1a14bc8
Compare
Populates the onoff server stubs in the Bluetooth Mesh sample, and
implements generic LED based hardware support to include more boards.
Fixes #31031.
Signed-off-by: Trond Einar Snekvik [email protected]