Skip to content
This repository was archived by the owner on Jul 24, 2022. It is now read-only.

Fix for ReadWriteMultipleRegisters#99

Open
yipilipi wants to merge 1 commit intoNModbus4:portable-3.0from
yipilipi:portable-3.0
Open

Fix for ReadWriteMultipleRegisters#99
yipilipi wants to merge 1 commit intoNModbus4:portable-3.0from
yipilipi:portable-3.0

Conversation

@yipilipi
Copy link

Just discover ReadWriteMultipleRegister not working. TransportPDU is correctly handled but the override can't be called because , need to override messageframe to call it.
Fix ModbusRTUTransport to handle reply. Need to check TCP and ascii
Add case for handling ReadWriteMultipeRegisiters

Add case for handling ReadWriteMultipeRegisiters
@codecov-io
Copy link

Current coverage is 63.56%

Merging #99 into portable-3.0 will increase coverage by +0.12% as of 0e0f4a1

@@            portable-3.0     #99   diff @@
============================================
  Files                 50      50       
  Stmts               2079    2086     +7
  Branches             192     192       
  Methods                0       0       
============================================
+ Hit                 1319    1326     +7
  Partial               24      24       
  Missed               736     736       

Review entire Coverage Diff as of 0e0f4a1

Powered by Codecov. Updated on successful CI builds.

@jikelmon
Copy link

Your fix works very well but i encountered that the read action is performed before the write action.
The modbus specification says that the write action has to be performed before the read action, doesn't it?
Would it be possible to change the behaviour of your patch to correspond with the official specification?

Best regards :)

@jacobbenlewis
Copy link

I made the same fix that I was about to create a pull request for until I saw this.

@jikelmon As far as the read/write action, isn't determined by the Slave? This request goes out as one message, the slave parses it and should do its write first, then respond with a read. Am I missing something?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants