ClassicDIY / ModbusTool

A modbus master and slave test tool with import and export functionality, supports TCP, UDP and RTU.
Apache License 2.0
640 stars 193 forks source link

Modbus slave "Write Single Register" function returns invalid PDU #5

Closed ThomasvLingen closed 6 years ago

ThomasvLingen commented 6 years ago

Hello once again, @graham22!

Similar to #4. A Non spec-compliant PDU is returned for the "Write Single Register" call when using the "Modbus Slave" tool.

Section 6.6 "06 (0x06) Write Single Register" of the Modbus Application Protocol document states the following:

The normal response is an echo of the request, returned after the register contents have been written.

Writing a value 0xAAAA to address 0x0000 using the "Write Single Register" Modbus function, the Modbus Slave tool sends me the following response (copied from the Communication log): TX: 00 01 00 00 00 06 01 06 00 00 00 01

Manually decoding it leads to the following:

Transaction ID: 0x0001
Protocol ID: 0x0000
Length: 0x0006
Unit ID: 0x01

Function Code: 0x06
Register Address: 0x0000
Register Value: 0x0001

The Register Value field is not equal to the written value (0xAAAA).

Going through the code, the bug seems to be in this file in the ServerEncode method. Instead of the written register value being pushed into the message, the message header is (using the PushRequestHeader function). This deviates from the specification.

graham22 commented 6 years ago

here is an update to that function, try replacing the attached ModbusLib.dll in C:\Program Files (x86)\SkyeTracker\ModbusSlave

    /// <summary>
    /// Append the typical header for a request command (master-side)
    /// </summary>
    /// <param name="command"></param>
    /// <param name="body"></param>
    internal static void PushRequestHeader(
        ModbusCommand command,
        ByteArrayWriter body)
    {
        body.WriteUInt16BE((ushort)command.Offset);
        if (command.FunctionCode == 05 || command.FunctionCode == 06)
        {
            body.WriteInt16BE((short)command.Data[0]);
        }
        else
        {
            body.WriteInt16BE((short)command.Count);
        }
    }

Graham.

On Tue, Dec 5, 2017 at 8:38 AM, Thomas van Lingen notifications@github.com wrote:

Hello once again, @graham22 https://github.com/graham22!

Similar to #4 https://github.com/graham22/ModbusTool/issues/4. A Non spec-compliant PDU is returned for the "Write Single Register" call when using the "Modbus Slave" tool.

Section 6.6 "06 (0x06) Write Single Register" of the Modbus Application Protocol document states the following:

The normal response is an echo of the request, returned after the register contents have been written.

Writing a value 0xAAAA to address 0x0000 using the "Write Single Register" Modbus function, the Modbus Slave tool sends me the following message (copied from the Communication log): TX: 00 01 00 00 00 06 01 06 00 00 00 01

Manually decoding it leads to the following:

Transaction ID: 0x0001 Protocol ID: 0x0000 Length: 0x0006 Unit ID: 0x01

Function Code: 0x06 Register Address: 0x0000 Register Value: 0x0001

The Register Value field is not equal to the written value (0xAAAA).

Going through the code, the bug seems to be in this https://github.com/graham22/ModbusTool/blob/master/ModbusLib/Protocols/Modbus/Codecs/ModbusCodecWriteSingleRegister.cs#L61 file in the ServerEncode method. Instead of the written register value being pushed into the message, the message header is (using the PushRequestHeader function). This deviates from the specification.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/graham22/ModbusTool/issues/5, or mute the thread https://github.com/notifications/unsubscribe-auth/AFGATTLfgEFK3W6j79QjmkkJ7CuwfynNks5s9UdXgaJpZM4Q2TUt .

-- Graham A. Ross (613) 769-1331

ThomasvLingen commented 6 years ago

Hi @graham22, Thanks for the reply!

Do you think we could make this into a change into the repository? That way, other people who may run into the same problem will have the bug fixed for them as well.

If you don't have the time, I could try to make a pull-request for this myself (although I am not very knowledgable when it comes to C#).

graham22 commented 6 years ago

Can you confirm that it now works?

On Wed, Dec 6, 2017 at 10:37 AM, Thomas van Lingen <notifications@github.com

wrote:

Hi @graham22 https://github.com/graham22, Thanks for the reply!

Do you think we could make this into a change into the repository? That way, other people who may run into the same problem will have the bug fixed for them as well.

If you don't have the time, I could try to make a pull-request for this myself (although I am not very knowledgable when it comes to C#).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/graham22/ModbusTool/issues/5#issuecomment-349676684, or mute the thread https://github.com/notifications/unsubscribe-auth/AFGATdndJUjaZGMnQ0K7SEH4jJFa5ohgks5s9rS3gaJpZM4Q2TUt .

-- Graham A. Ross (613) 769-1331

ThomasvLingen commented 6 years ago

Hi @graham22,

I've tested your new commit and it has fixed the problem for this issue. However, #4 was not fixed by this (since this writes back the decoded value from a ModbusCommand, which had a different representation than 0x0000 or 0xFF00 due to the way it was decoded).

I have submitted a pull request for this #6, this should fix #4 as well.

As always, thanks for taking the time to respond!