Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modbus Flex Sequencer broken in 5.26? #420

Open
hoegge opened this issue Oct 28, 2023 · 6 comments
Open

Modbus Flex Sequencer broken in 5.26? #420

hoegge opened this issue Oct 28, 2023 · 6 comments
Assignees
Labels
community could be done by the community help wanted question

Comments

@hoegge
Copy link

hoegge commented Oct 28, 2023

Which node-red-contrib-modbus version are you using?

5.26 - newest in node-red pallete

What happened?

After upgrading from 5.23 all my Modbus flows has stopped. Nothing gets through the Mobus Flex Sequencer It simply does nothing. The Modbus Flex Getter works fine.

It seems after a lot of troubleshooting, that the Flex Sequencer only fires if msg.payload contains something? Otherwise it ignores input.

Much slower too

Furthermore 5.26 is MUCH slower reading data using the sequencer - around a factor 5-10 compared to version 5.23. Speed instantly increased again going back to 5.23.

Server

Other/External server

How can this be reproduced?

Use in in Node-red 3.0.2

What did you expect to happen?

No response

Other Information

No response

@hoegge hoegge added the bug label Oct 28, 2023
@hoegge
Copy link
Author

hoegge commented Oct 28, 2023

It seems to have happened here: b7acd97
where this was introduced here: 758a3a5#r131145217

Seems to be a breaking change, since documentation does not require an msg.payload input but msg.messages

@grewek grewek self-assigned this Dec 4, 2023
@grewek
Copy link

grewek commented Dec 5, 2023

Hi @hoegge

Thank you for the report, i have looked at the first issues and it seems to me that this is intended behaviour as we do not want the node to work on an empty payload.

@biancode biancode added help wanted question community could be done by the community and removed bug labels Jan 27, 2024
@biancode
Copy link
Contributor

biancode commented Jan 28, 2024

You can inject a timestamp and not empty msg and msg.payload and it works in our testing as expected.

@hoegge
Copy link
Author

hoegge commented Jan 28, 2024

Ok / thanks for clarification @grewek and @biancode - But why do you want a msg.payload when the documentation only says you want a msg.messages. Then this should be added as a required item in the docs? It is a breaking change - so should at least be documented. I have tried to submit clarified documentation to this project before just to be rejected :-(

@biancode
Copy link
Contributor

biancode commented Jan 31, 2024

@hoegge thank you for the question and we had not enough time invested until know to change that node to our defaults. At the moment the code says it still msg.sequences and payload is checked from a first change from us as all nodes do the check. Next step would be to set sequences into msg.payload. I think that node needs a bit more refactoring work to stay compatible to existing flows.

Copy link

github-actions bot commented Apr 1, 2024

This issue is stale because it has been open 60 days with no activity. It will be closed in 30 days, but can be saved by removing the stale label or commenting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community could be done by the community help wanted question
Projects
None yet
Development

No branches or pull requests

4 participants