search

grid-x / modbus

BSD 3-Clause "New" or "Revised" License
78 stars 26 forks source link

fix: retry reading query response on transaction id mismatch #49

Closed guelfey closed 2 years ago

guelfey commented 2 years ago

This was previously attempted with the flushAll call, but this doesn't seem to be reliable in practice - since it sets the read timeout to time.Now(), this means that in practice the Read in most cases actually returns directly without attempting to read.

Instead of trying to extend the deadline for the flushing read by some arbitrary small value, this fix handles the problem at a different place: if we detect a transaction ID mismatch by 1, we simply ignore the response and assume that a matching response will follow. This required some refactoring of the reading code since just retrying the read without another write wasn't easily possible.

guelfey commented 2 years ago

I see it as a bug fix actually - this was supposed to already happen

gq0 commented 2 years ago

I see it as a bug fix actually - this was supposed to already happen

The transaction ID handling seems new to me. But we can also roll it out slowly and see 👍🏻