refactor: use a error handling similar to CAN #8

higor-melo · 2025-12-04T13:58:50Z

Depends on:

What is this PR for

Implement a CAN Similar Error Handling

How I did it

Results, How I tested

Checklist

Assign yourself in the PR
Write unit tests (when relevant)
Run rubocop and rake tests locally
If this PR initialize a CMAKE package please enable styling and linting
Analyse if this PR modifies the UI, if so:
- Tested Tupan's simulation (Charts) 🗺️
- Tested Tupan's simulation (Hud) 🕹️
- Tested ROV's simulation 🕹️

doudou

Still missing handling of "bad length", which is actually the one error we currently have.

And no unit tests.

src/RTUMaster.cpp

src/RTUMaster.hpp

wvmcastro

fix: do error handling similarly to CAN is actually IMO a feat
and feat: store validation on it's on method to be used elsewhere
is a refactor or at least a chore

src/RTUMaster.cpp

src/RTUMaster.hpp

test/test_RTUMaster.cpp

src/RTUMaster.hpp

src/common.cpp

src/RTUMaster.cpp

jhonasiv

The only thing I didnt check was whether the expected packet length is correct for each method. Since you didnt explicitly change any tests regarding this, I think they are right, but this is not passing CI at moment with errors on motors_weg_cvw300, so maybe not. Waiting for updates on that

higor-melo · 2025-12-16T16:21:58Z

The only thing I didnt check was whether the expected packet length is correct for each method. Since you didnt explicitly change any tests regarding this, I think they are right, but this is not passing CI at moment with errors on motors_weg_cvw300, so maybe not. Waiting for updates on that

Yep, there was indeed an issue with one of the tests. I'm double checking all the tests using the modbus reference from here: https://www.modbustools.com/modbus.html

src/common.cpp

Keep the default constructor as initializing everything with 1 for threshold and increment

AFAIK this is not used, I was able to build everything and it all worked fine. So i'll be removing it and all the tests related to it.

add a new test for UnexpectedReply function code and length

…essage Therefore the response payload should have 4 bytes of size corresponding to the Address High, Address Low, Data High, Data Low

We're assuming the errors won't happen as much

src/MasterInterface.hpp

src/RTUStatistics.hpp

src/RTUMaster.cpp

.vscode/c_cpp_properties.json

src/RTUStatistics.hpp

remove c_cpp_properties.json from git tracked files typo `sucess` to `success`

src/RTUStatistics.hpp

Testing/Temporary/CTestCostData.txt

Testing/Temporary/LastTest.log

higor-melo requested review from doudou and wvmcastro December 4, 2025 13:58

higor-melo self-assigned this Dec 4, 2025

doudou reviewed Dec 8, 2025

View reviewed changes

src/RTUMaster.cpp Outdated Show resolved Hide resolved

src/RTUMaster.cpp Outdated Show resolved Hide resolved

doudou reviewed Dec 8, 2025

View reviewed changes

src/RTUMaster.cpp Outdated Show resolved Hide resolved

src/RTUMaster.cpp Outdated Show resolved Hide resolved

doudou reviewed Dec 8, 2025

View reviewed changes

src/RTUMaster.cpp Outdated Show resolved Hide resolved

src/RTUMaster.hpp Outdated Show resolved Hide resolved

higor-melo requested a review from doudou December 8, 2025 20:52

higor-melo commented Dec 8, 2025

View reviewed changes

src/RTUMaster.hpp Outdated Show resolved Hide resolved

wvmcastro reviewed Dec 9, 2025

View reviewed changes

src/RTUMaster.cpp Outdated Show resolved Hide resolved

src/RTUMaster.hpp Show resolved Hide resolved

src/RTUMaster.hpp Outdated Show resolved Hide resolved

higor-melo changed the title ~~[DRAFT] fix: allow retries according to error count on modbus~~ [DRAFT] refactor: use a error handling similar to CAN Dec 9, 2025

higor-melo requested a review from wvmcastro December 9, 2025 13:52

doudou reviewed Dec 10, 2025

View reviewed changes

src/RTUMaster.hpp Outdated Show resolved Hide resolved

src/RTUMaster.hpp Outdated Show resolved Hide resolved

test/test_RTUMaster.cpp Outdated Show resolved Hide resolved

src/RTUMaster.hpp Outdated Show resolved Hide resolved

higor-melo requested review from doudou, jhonasiv and seixasxbr and removed request for wvmcastro December 15, 2025 13:21

higor-melo force-pushed the modbus-error-handling branch from 682930a to 27caf70 Compare December 15, 2025 16:11

higor-melo marked this pull request as ready for review December 15, 2025 16:14

higor-melo changed the title ~~[DRAFT] refactor: use a error handling similar to CAN~~ refactor: use a error handling similar to CAN Dec 15, 2025

higor-melo mentioned this pull request Dec 15, 2025

feat: report RTU statistics tidewise/drivers-orogen-motors_weg_cvw300#25

Open

10 tasks

higor-melo force-pushed the modbus-error-handling branch 3 times, most recently from d561cfe to 216ad13 Compare December 15, 2025 18:04

doudou reviewed Dec 15, 2025

View reviewed changes

higor-melo requested a review from doudou December 15, 2025 20:06

jhonasiv reviewed Dec 16, 2025

View reviewed changes

higor-melo force-pushed the modbus-error-handling branch from e2a733a to ef009d3 Compare December 16, 2025 13:27

higor-melo requested a review from jhonasiv December 16, 2025 16:22

seixasxbr reviewed Dec 17, 2025

View reviewed changes

src/common.cpp Outdated Show resolved Hide resolved

src/common.cpp Outdated Show resolved Hide resolved

higor-melo added 12 commits December 17, 2025 16:00

feat: store validation on it's on method to be used elsewhere

9c95bf6

feat: validate packets and retry when needed

d369a3b

feat: allow thresholds to be decided

b4c268e

feat: allow setting values on initialization

cbf56c3

Keep the default constructor as initializing everything with 1 for threshold and increment

fix: validate registers reply for all functions

88c015d

feat: report errors through a struct

4187182

chore: remove request method since it's not used

08f8f26

AFAIK this is not used, I was able to build everything and it all worked fine. So i'll be removing it and all the tests related to it.

fix: update total sucess count

257c97e

add a new test for UnexpectedReply function code and length

chore: code improvements

e1e0fd9

chore: remote include <cmath> and remove floating point

9799777

fix: the response from the writeSingleRegister is the echo from the m…

ee21e09

…essage Therefore the response payload should have 4 bytes of size corresponding to the Address High, Address Low, Data High, Data Low

chore: use int for counting

bd17faf

We're assuming the errors won't happen as much

higor-melo force-pushed the modbus-error-handling branch from 5f539e9 to bd17faf Compare December 17, 2025 19:08

feat: re-add check against the byte_count

96dd3d4

higor-melo requested review from doudou and seixasxbr December 17, 2025 19:32

seixasxbr reviewed Dec 17, 2025

View reviewed changes

higor-melo requested a review from seixasxbr December 17, 2025 20:24

chore: improve member initialization

15a2d3a

doudou reviewed Dec 18, 2025

View reviewed changes

.vscode/c_cpp_properties.json Outdated Show resolved Hide resolved

src/RTUStatistics.hpp Outdated Show resolved Hide resolved

src/RTUStatistics.hpp Outdated Show resolved Hide resolved

src/RTUStatistics.hpp Outdated Show resolved Hide resolved

higor-melo requested a review from doudou December 18, 2025 12:45

chore: define fixed-sized integer types

4b0912d

remove c_cpp_properties.json from git tracked files typo `sucess` to `success`

higor-melo force-pushed the modbus-error-handling branch from 624d59a to 4b0912d Compare December 18, 2025 13:05

doudou reviewed Dec 18, 2025

View reviewed changes

src/RTUStatistics.hpp Outdated Show resolved Hide resolved

Testing/Temporary/CTestCostData.txt Outdated Show resolved Hide resolved

Testing/Temporary/LastTest.log Outdated Show resolved Hide resolved

higor-melo requested a review from doudou December 18, 2025 17:25

higor-melo force-pushed the modbus-error-handling branch 2 times, most recently from 22db9c4 to 1833dc5 Compare December 18, 2025 17:57

chore: improve commentary and remove testing files that got added

0f1ae27

higor-melo force-pushed the modbus-error-handling branch from 1833dc5 to 0f1ae27 Compare December 18, 2025 18:04

seixasxbr approved these changes Dec 19, 2025

View reviewed changes

refactor: use a error handling similar to CAN #8

Are you sure you want to change the base?

refactor: use a error handling similar to CAN #8

Uh oh!

Conversation

higor-melo commented Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is this PR for

How I did it

Results, How I tested

Checklist

Uh oh!

doudou left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wvmcastro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhonasiv left a comment

Choose a reason for hiding this comment

Uh oh!

higor-melo commented Dec 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

higor-melo commented Dec 4, 2025 •

edited

Loading

doudou left a comment •

edited

Loading