Feature/ssr pr2 wpc integration#150
Conversation
| app_res_e WPC_initialize(const char * port_name, unsigned long bitrate) | ||
| { | ||
| wpc_ssr_reset(); | ||
| int res = WPC_Int_initialize(port_name, bitrate); |
There was a problem hiding this comment.
This comment is more of a reminder for later, not really relevant to this commit :)
This one starts a thread for getting the indications. The thread waits 500ms in the beginning before polling indications so in practice it should be fine - we shouldn't get an MSAP SSR registration indication before the ssr functions below.
I think the historical reason for that delay is to make sure dbus is initialized in the sink service so we can push the received data packets.
But now there are 2 things justifying such a delay and it might be good to implement some mechanism to start the indication polling thread separately, manually after WPC_initialize() is called.
| { | ||
| // Stopping the stack reboots the device, so all learned SSR routes | ||
| // become stale and must be rebuilt from fresh registrations. | ||
| wpc_ssr_reset_routes(); |
There was a problem hiding this comment.
This might be needed to be added to msap_stack_state_indication_handler as well, for example if the stack is restarted via remote api, WPC_stop_stack is not called.
Reading the node role and address for SSR can be good too in that handler, if those are updated from remote api too. At least remote api documentation says those CSAP attributes need a reboot after updating.
| { | ||
| uint32_t first_hop_id_le; | ||
|
|
||
| uint32_encode_le(*first_hop_id, (uint8_t *) &first_hop_id_le); |
There was a problem hiding this comment.
If this is for the ordering of the bytes for the final message, I would prefer having this right before final dualmcu api message is built, in fill_tx_ssr_request for example.
Not sure if the library actually runs on any big endian system, but when we receive an array of uint32_t in any function such as dsap_data_tx_ssr_request, it's less confusing if the elements have native byte ordering (for debugging etc).
| * See file LICENSE for full license details. | ||
| * | ||
| */ | ||
| #define LOG_MODULE_NAME "wpc" |
There was a problem hiding this comment.
duplicate module name, can be wpc_ssr. Not a problem now since no logs are used here.
| Platform_unlock_ssr(); | ||
| } | ||
|
|
||
| void wpc_ssr_on_role_set(app_role_t role) |
There was a problem hiding this comment.
Same function as wpc_ssr_on_role_read, can be renamed and reused.
|
|
||
| gtest_discover_tests(meshAPI_wpc_internal_unit) | ||
|
|
||
| add_executable(meshAPI_wpc_unit |
There was a problem hiding this comment.
A bit misleading executable name, can add ssr to it.
| EXPECT_EQ(g_stub.ssr_init_calls, 1); | ||
| ASSERT_EQ(g_stub.sink_addresses.size(), 1u); | ||
| EXPECT_EQ(g_stub.sink_addresses.front(), 0x11223344u); | ||
| EXPECT_EQ(g_stub.ssr_clear_sink_address_calls, 2); |
There was a problem hiding this comment.
With traditional development, such specific checks might be excessive even though I tend to add them too :D But now probably not a problem to update unit tests with LLMs, for example if the number of internal clear calls changes.
We can also add gmock, it might make these kind of stub checks easier. Overall I love that there are now real unit tests that can be ran without a sink, this is great!
mortewm
force-pushed
the
feature/ssr-pr1-foundation
branch
from
a73abc2 to
2fd4cfe
Compare
2 participants
This PR builds on the SSR foundation PR and integrates SSR into WPC lifecycle
handling and data sending.
With this change, WPC tracks the information SSR needs during initialization and
shutdown, and
WPC_send_data_with_options()can automatically use SSR when aroute is available.
WPC_initialize,WPC_close, andWPC_stop_stackWPC_send_data_with_options()when a route exists