Read timeouts in fsmpico#4
Open
saebischer wants to merge 2 commits intomasterfrom
Open
Conversation
This commit is a transfer of GitLab branch 88-read-timeouts-in-fsm.
dettonijr
requested changes
Jan 31, 2018
| // Defines | ||
|
|
||
| #define READ_TIMEOUT (5000) | ||
| #define CONTINUOUS_TIMEOUT_LEEWAY (1000) |
Member
There was a problem hiding this comment.
Can we unify those two constants in a single leeway? They seem to have similar semantics.
| fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); | ||
| stateTransition(fsmpico, FSMPICOSTATE_SERVICEREAUTH); | ||
| // set a timeout for awaiting a response | ||
| fsmpico->comms->setTimeout(fsmpico->reauthDelay + CONTINUOUS_TIMEOUT_LEEWAY, fsmpico->user_data); |
Member
There was a problem hiding this comment.
There is some inconsistence in this timeout.
Service sends reauth 10 seconds
Pico replies back and set a timeout to 10s + LEEWAY
Service sends reauth 10 seconds
Pico waits for 10 seconds
Pico replies back and set a timeout to 10s + LEEWAY
The timeouts should expire in the same point in time.
Remove the timeout between reading a ServiceReauth message and sending the PicoReauth message.
llewelld
reviewed
Feb 6, 2018
| fsmpico->comms->write(buffer_get_buffer(message), buffer_get_pos(message), fsmpico->user_data); | ||
| stateTransition(fsmpico, FSMPICOSTATE_STATUS); | ||
| // set a timeout for waiting for the status message | ||
| fsmpico->comms->setTimeout(READ_TIMEOUT, fsmpico->user_data); |
Member
There was a problem hiding this comment.
To match the pattern of fsmservice, the timeout on connect is sufficient. This timeout should be removed, unless there's a good reason for it that I'm missing?
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Transfer of merge request !90 from GitLab.