modbus: reset wait semaphore before tx #79345
Merged
Conversation
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
legoabram
added
backport v3.5-branch
backport v3.7-branch
A response returned after a request times out would increment the semaphore and stay until the next request is made which will immediately return when k_sem_take is called even before a response is returned. This will once again have the same problem when the actual response arrives. So the wait semaphore just needs to be reset before transmitting. Signed-off-by: Abram Early <abram.early@gmail.com>
legoabram
force-pushed
the
abram/modbus-reset-sem
branch
from
1b2060a to
add7c4c
Compare
jfischer-no
approved these changes
Oct 7, 2024
|
|
||
| int modbus_tx_wait_rx_adu(struct modbus_context *ctx) | ||
| { | ||
| k_sem_reset(&ctx->client_wait_sem); |
Contributor
There was a problem hiding this comment.
Would you like to add a test case to catch this in the tests?
Contributor
Author
There was a problem hiding this comment.
If I get some leeway at work, I'll get an nucleo board working and submit a test case. I think the easiest way to do it would be to artificially delay the server response past the timeout. But I'm not sure when I'll get to it.
jfischer-no
requested review from
bjarki-andreasen,
carlescufi and
fabiobaltieri
fabiobaltieri
approved these changes
Oct 7, 2024
This was referenced Oct 7, 2024
Contributor
Author
|
Tagged this merge with a bug report for tracking purposes, and the backports. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
A response returned after a request times out would increment the semaphore and stay until the next request is made which will immediately return when k_sem_take is called even before a response is returned. This will once again have the same problem when the actual response arrives. So the wait semaphore just needs to be reset before transmitting.
Ideally, I'd like to have the modbus test check for this edge case, but I don't have an frdm-k64f board, or the time to figure out how to configure a new board for the test to develop one. That said, I'm hoping this is simple and straightforward enough to get the point across. I've been running this modification on our boards for several days now and it's only improved behavior. But anecdotes only go so far.
Fixes #79518