Skip to content

Issue #3849 Fix#3852

Open
JTrantow wants to merge 2 commits intoLinuxCNC:masterfrom
JTrantow:master
Open

Issue #3849 Fix#3852
JTrantow wants to merge 2 commits intoLinuxCNC:masterfrom
JTrantow:master

Conversation

@JTrantow
Copy link
Contributor

@JTrantow JTrantow commented Mar 9, 2026

Fix for calc_ifdelay() and ENOMSG message handling.
Without the ENOMSG fix, the modbus would lock up upon a single ENOMSG error.
The calc_ifdelay() fix corrects the delay calculation for baud rates > 19.2K. Without this fix, you are more likely to get into the ENOMSG condition.

@BsAtHome
Copy link
Contributor

BsAtHome commented Mar 9, 2026

This statement:

#define HARDWARE_MAX_DELAY (1020)	//!< Maximum ifdelay in number of bits limited by hardware.

Is only partly correct. The max delay (in hardware) is 255. However, PktUART v3 has a x4 clock divider on it that v2 does not have (older than v2 is not supported). So, stating that the max delay is 1020 is only valid for one version of the hardware and not the other. The difference is accounted for elsewhere (and is clamped in the hm2_pktuart_config() implementation).

@JTrantow
Copy link
Contributor Author

JTrantow commented Mar 9, 2026

What do you think about removing the 1020 check from the calc_if() completely and have the hardware limitation get enforced when it gets applied at the PktUART level? That would simplify calc_if() to modbus bit timing without the hardware implementation constraint and allow the PktUART version to clamp/max the value appropriately.

@BsAtHome
Copy link
Contributor

BsAtHome commented Mar 9, 2026

Well, you made a four-line diff I suggested into a larger issue (see #3849 (comment)). I'm not sure that is wise.

If you want to change how the component is written, commented and documented, then I'd be fine with that and you are very welcome to do so and create a PR for that. But the extra changes have no value in addressing the bug identified because it conflates two different issues. That is my point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants