bristlemouth / bm_protocol

Primary Bristlemouth firmware repository
https://www.bristlemouth.org/
Apache License 2.0
10 stars 7 forks source link

Feature/bm serial get bcmp cfgs #93

Closed victorsowa12 closed 6 months ago

victorsowa12 commented 6 months ago

Using the node_id and adding callbacks to handle config req/rep! This allows Spotter to set/get/del configs from its CLI. I also added that we call the callback upon bcmp request message timeout. So I made sure the callbacks can handle a NULL payload ~however, for the handling of a failed request right now I am just doing a bm_printf~. I changed it to use a BRIDGE_CFG_LOG_PRINT of the failure.

Example from Spotter's CLI:

bm topo
Bristlemouth toplogy: 4d58ce2f22dfb374 | 910030066a2926a7 | 193e6faff528b841
bm cfg status 910030066a2926a7 s
Succesfull status request send
2024-02-28T22:09:00.625Z [BRIDGE_CFG] [INFO] Response msg -- Node Id:910030066a2926a7,Partition:system, Commit Status:0
2024-02-28T22:09:00.625Z [BRIDGE_CFG] [INFO] Num Keys: 1
2024-02-28T22:09:00.625Z [BRIDGE_CFG] [INFO] Key 0: sensorsPollIntervalMs
2024-02-28T22:09:00.640Z [BRIDGE_CFG] [INFO] Node ID: 910030066a2926a7 Partition: system Value: 0

bm cfg get 910030066a2926a7 s sensorsPollIntervalMs
Succesfully sent config get msg
2024-02-28T22:09:11.515Z [BRIDGE_CFG] [INFO] Node ID: 910030066a2926a7 Partition: system Value: 0

bm cfg status 193e6faff528b841 s
Succesfull status request send
2024-02-28T22:09:17.617Z [BRIDGE_CFG] [INFO] Response msg -- Node Id:193e6faff528b841,Partition:system, Commit Status:0
2024-02-28T22:09:17.617Z [BRIDGE_CFG] [INFO] Num Keys: 1
2024-02-28T22:09:17.617Z [BRIDGE_CFG] [INFO] Key 0: sensorsPollIntervalMs
2024-02-28T22:09:17.632Z [BRIDGE_CFG] [INFO] Node ID: 193e6faff528b841 Partition: system Value: 0

bm cfg get 193e6faff528b841 s sensorsPollIntervalMs
Succesfully sent config get msg
2024-02-28T22:09:40.687Z [BRIDGE_CFG] [INFO] Node ID: 193e6faff528b841 Partition: system Value: 0

bm cfg set 910030066a2926a7 s u sensorsPollIntervalMs 5000
Succesfully sent config set msg
2024-02-28T22:14:33.937Z [BRIDGE_CFG] [INFO] Node ID: 910030066a2926a7 Partition: system Value: 5000

bm cfg commit 910030066a2926a7 s
Succesfull config commit send

bm cfg get 910030066a2926a7 s sensorsPollIntervalMs
Succesfully sent config get msg
2024-02-28T22:15:15.882Z [BRIDGE_CFG] [INFO] Node ID: 910030066a2926a7 Partition: system Value: 5000

bm cfg del 910030066a2926a7 s sensorsPollIntervalMs
Successfully sent del key request
2024-02-28T22:17:04.757Z [BRIDGE_CFG] [INFO] Response msg -- Node Id:910030066a2926a7,Partition:system, Key: sensorsPollIntervalMs, Success: 1

bm cfg commit 910030066a2926a7 s
Succesfull config commit send

bm cfg status 910030066a2926a7 s
Succesfull status request send
2024-02-28T22:17:48.453Z [BRIDGE_CFG] [INFO] Response msg -- Node Id:910030066a2926a7,Partition:system, Commit Status:0
2024-02-28T22:17:48.453Z [BRIDGE_CFG] [INFO] Num Keys: 0
victorsowa12 commented 6 months ago

I realized since I had to merge develop into the feature/bm_serial_get_bcmp_cfgs branch for the BRIDGE_CFG_LOG_PRINT function there are a bunch of changes on develop that are now also being merged into feature/bcmp_req_rep. So I highlighted the two files to look at with comments, but those are bcmp.cpp and ncp_config.cpp.

towynlin commented 6 months ago

Sorry for the duplicate comment... something to do with using github's in-browser code editor to do the review. 🤷

victorsowa12 commented 6 months ago

Yes that flow is correct!

does bm_serial call ncp_cfg_get_cb in the bridge? And perhaps that used to be the only place that would be called, so it was unconditional.

This is still true, ncp_cfg_get_cb is still only called from bm_serial. The if node_id == 0 || node_id == getNodeID() just decides whether we respond directly with the bridges configs, or send out a request to another node using bcmp_config_get. The "_cb" is used to tell bcmp_config_get that when it receives a reply, forward the reply via bm_serial back to Spotter.

towynlin commented 6 months ago

I see, yeah, perfect, thank you. So really — the core feature we were aiming for is in that else clause. It used to ignore the node id and only work for the bridge directly. Now we actually check the node id, and if it's for someone else, we make the bcmp request. ✨