Fix libmodbus build for mingw #3
Open
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
jimklimov
force-pushed
the
fix-mingw-build
branch
3 times, most recently
from
6ef9d67 to
f64103c
Compare
|
Ooooh that was an adventure to get builds and |
jimklimov
commented
Jan 18, 2025
src/modbus.c
Outdated
| } | ||
| // else: We have at most tried some default FD's but not | ||
| // the (lacking) one for the backend, so fall through for | ||
| // its recv method anyway (e.g. query libusb directly). |
There was a problem hiding this comment.
The last commit regards networkupstools/nut#2609 (comment) and follows up from PR #1:
I also feel a bit stupid, as I overlooked that the method involved in
_modbus_receive_msg()is not a standardselect(), but a specificctx->backend->select()so the one here_modbus_rtu_usb_select()does not care aboutrsetand talks to the libusb context. The fixes in that PR to avoidFD_SET(-1)are valid (to avoid illegal memory access), but are not the full answer here as I hoped it could be.
This was referenced Jan 26, 2025
Merged
jimklimov
force-pushed
the
fix-mingw-build
branch
from
f0ae6be to
e33dc9e
Compare
…() to auto-flush so logs are comprehensible Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… console logs on WIN32 Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…> 5ms" test case It fails or passes randomly on different platforms Win32/mingw, FreeBSD, MacOS X maybe something remains in buffer? We'll see if extra sleep helps. Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…isters" so the buffer does not contaminate the result on some systems Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… "33ms > 20ms" Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
…d OpenBSD Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… their own category Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… > Yms)" to increase the timeouts and fit all laggy systems better Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
… on WIN32, and we follow MODBUS_ERROR_RECOVERY_LINK, do modbus_flush(ctx) At least this allows unit-tests to pass... Signed-off-by: Jim Klimov <jimklimov+nut@gmail.com>
jimklimov
force-pushed
the
fix-mingw-build
branch
from
e33dc9e to
db24fed
Compare
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.
Breakage was introduced by upstream for modbus-tcp (some time after v3.1.7 that built well), and by
ssize_tprinting from modbus-rtu-usb.Fixes verified by:
x86_64-w64-mingw32),x86_64-w64-mingw32andi686-w64-mingw32targets (with gcc), andNo warnings/errors emitted by either scenario.
UPDATE: While trying to check this on other NUT CI farm workers (OpenBSD, FreeBSD, MacOS, OmniOS, OpenIndiana...) a few recurrent issues popped up and also were fixed in this branch.
UPDATE: Piece by piece, parts of this PR are extracted into smaller PRs with a more focused scope of changes, proposed for this NUT fork of libmodbus as well as for upstream (where easily applicable). It is being rebased as those less-questionable change sets get merged into the fork, so I expect it to become a null change soon and be just closed.
While the remaining code of this PR mostly concerns tests (and specifically flakiness of the test case with small timeouts), and console-flushing needed when running verbosely on Windows, there is also a questionable change about flushing the modbus connection when retrying. Not sure if it needs be.