Lwm2m boostrap fixes #1271
Merged
Lwm2m boostrap fixes #1271
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SeppoTakalo
force-pushed
the
lwm2m_boostrap_nrf
branch
from
bbf8513
to
d310068
Compare
Engine wake-up call was missing from pause/resume APIs which caused delay. Upstream PR: zephyrproject-rtos/zephyr#61370 Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no> (cherry picked from commit cb46ff519aa7e99f96955b1150a8d3fd54925a75)
…_state() Some state changes were do by directly writing into client.engine_state variable, followed by call next_event_at(0); This causes hard-to-find side effects. Refactor all state transitions to use set_sm_state() to have better control for it. Upstream PR: zephyrproject-rtos/zephyr#61370 Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no> (cherry picked from commit b55a3b9ca9de116e87a71b1ace05e3ded9dae055)
…tstrap When BOOTSTRAP FINNISH message was received, it caused engine to immediately switch to BOOTSTRAP_TRANS_DONE state which then closed the connection. Ack packet was still on the send-queue so it never got send before close(). Upstream PR: zephyrproject-rtos/zephyr#61370 Signed-off-by: Seppo Takalo <seppo.takalo@nordicsemi.no> (cherry picked from commit 42c0a915d0b6565159ed91dc687b72afe62ad2bb)
SeppoTakalo
force-pushed
the
lwm2m_boostrap_nrf
branch
from
d310068
to
988a149
Compare
rlubos
approved these changes
Aug 10, 2023
jheiskan81
approved these changes
Aug 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Original PR: zephyrproject-rtos/zephyr#61370
Related nrf PR: nrfconnect/sdk-nrf#12006
This PR fixes major bootstrap related issue and few minor issues.
Bootstrap issue
After introducing a tickless state-machine, our client now enters BOOTSTRA_TRANS_DONE immeadialy when Bootstrap-finnish packet is received. The state transition causes existing socket to be closed, and enters a next state which is DO_REGISTRATION. The problem is that ACK packet to Finnish message was still on the send-queue.
This is a problem on some bootstrap server, as the Finnish is confirmable message, and it may consider bootstrap to be failed as we just closed the connection without ACK.
Fix the issue by introducing sm_set_state_delayed() function that causes the next state transition to be delayed. The same delay was previously a side, effect of slow socket-loop. (There is a random possibility, that it did not work previously, but Wireshark shows that it does).
This fix alone was not enough, as some callback modified the next event timestamp directly.
Do not directly modify state variables
On many places in RD client. ctx.client_state was directly written, followed by call next_event_at(0).
I removed all these, and refactored it to use set_sm_state(). Notable change was the lwm2m_rd_client_connection_resume() as it modified the timestamp unconditionally, and without this fix, the previous one does not work.
Wake up engine on pause/resume
Engine pause and Engine resume are public API, so instead of waiting for event-loop to wake up, they should cause it to immediately wake up, so there would be no delay.
This is minor fix, and nobody was complaining it yet.