fix: send_ack_packet no longer causes unexpected behaviour if shared data lib fails to send #64
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
behaviour if the Shared_Data_sendData function fails.
GwendalRaoul
reviewed
Jun 5, 2023
| @@ -228,23 +228,25 @@ static uint32_t send_ack_packet(void) | |||
| res = Shared_Data_sendData(&data_to_send, packet_sent_cb); | |||
| if (res != APP_LIB_DATA_SEND_RES_SUCCESS) | |||
| { | |||
| LOG(LVL_WARNING, "State WAIT_DATA : Error sending ACK (res:%d).", res); | |||
| return APP_SCHEDULER_SCHEDULE_ASAP; | |||
| LOG(LVL_WARNING, "State WAIT_DATA : Error sending ACK (res:%u).", res); | |||
There was a problem hiding this comment.
I think setting "m_events.timeout = 1;" here instead would be cleaner.
It should have exactly same behavior.
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.
Hi,
This is a pull request to fix a bug contained in the provisioning library which can cause the provisioning library to exit early and enter an invalid state.
Bug
When a failure occurs while sending an
ackpacket to the provisioning server using the shared data library the failure is handled by returningAPP_SCHEDULER_SCHEDULE_ASAP. This causes the state machine to exit early and the library to be "stuck" in the state. Consequently, the initiator's end of provisioning callback is never called so it may stall the behaviour of their system awaiting this call.Furthermore, the code explicitly states in a comment that the ACK is optional and in the event of a failure the provisioning process should move to the next state; which it does not in this case.
Proposed fix
The fix proposed in this pull request instead handles this as an "immediate timeout". This causes a timeout event to occur and the system to proceed as usual.
An alternative fix could be to explicitly set
m_events.timeout = 1instead of starting the timeout task (which does this).