7

I am using C# to communicate via modbus rs485 rs232 to 2 phase meters that among other log the power voltage.

I have to send data over the bus so that i can receive the readings.
I have connected a normal wire and shorted the send and receive.

The data is recieved and this event is fired:

private void port_DataReceived(object sender, SerialDataReceivedEventArgs e)
{
    SerialPort sp = (SerialPort)sender;
    byte[] buff = new byte[sp.BytesToRead];

    //Read the Serial Buffer
    sp.Read(buff, 0, buff.Length);
    string data= sp.ReadExisting();

    foreach (byte b in buff)
    {
        AddBuffer(b); //Add byte to buffer
    }
}

Then this buffer is sent to another function which is this one:

private void AddBuffer(byte b)
{
    buffer.Add(b);

    byte[] msg =  buffer.ToArray();

    //Make sure that the message integrity is correct
    if (this.CheckDataIntegrity(msg))
    {
        if (DataReceived != null)
        {
            ModbusEventArgs args = new ModbusEventArgs();
            GetValue(msg, args);
            DataReceived(this, args);
        }
        buffer.RemoveRange(0, buffer.Count);

    }
}

I think that the problem lies at the data integrity check:

public bool CheckDataIntegrity(byte[] data)
{
    if (data.Length < 6)
        return false;
    //Perform a basic CRC check:
    byte[] CRC = new byte[2];
    GetCRC(data, ref CRC);
    if (CRC[0] == data[data.Length - 2] && CRC[1] == data[data.Length - 1])
        return true;
    else
        return false;
}

There is a CRC check and what is strange is that it never becomes true. The CRC calculation:

private void GetCRC(byte[] message, ref byte[] CRC)
{

    ushort CRCFull = 0xFFFF;
    byte CRCHigh = 0xFF, CRCLow = 0xFF;
    char CRCLSB;

    for (int i = 0; i < (message.Length) - 2; i++)
    {
        CRCFull = (ushort)(CRCFull ^ message[i]);

        for (int j = 0; j < 8; j++)
        {
            CRCLSB = (char)(CRCFull & 0x0001);
            CRCFull = (ushort)((CRCFull >> 1) & 0x7FFF);

            if (CRCLSB == 1)
                CRCFull = (ushort)(CRCFull ^ 0xA001);
        }
    }
    CRC[1] = CRCHigh = (byte)((CRCFull >> 8) & 0xFF);
    CRC[0] = CRCLow = (byte)(CRCFull & 0xFF);
}
default
  • 11,485
  • 9
  • 66
  • 102
Combinu
  • 882
  • 2
  • 10
  • 31
  • Are there online resource that have a transcript of a typical communication session, complete with the CRC? Then you could at least apply your algorithm to those example messages and see if you come up with the same CRC. – Andyz Smith Aug 28 '13 at 12:55
  • 1
    What is the "buffer" variable? Is it a List? Are you sure that your "msg" variable is ever larger than 6? Why not just use the buffer off the Serial Port instead of breaking it down in a loop by byte, reconstructing it in a List, then converting it back to a global byte array? You also call ReadExisting on the Serial Port immediately after reading the contents of the buffer, why? – B L Aug 28 '13 at 12:55
  • The CRC seems to be correct @AndyzSmith. what do you mean calling readexisting realy where can i call it? and yes buffer is a list – Combinu Aug 28 '13 at 12:59
  • Or better yet, is there some sort of terminating character you can use to signify the end of a valid data transmission instead of accumulating bytes ad infinitum and checking a length? – B L Aug 28 '13 at 13:06
  • After your sp.Read() line, you immediately call sp.ReadExisting(). It's unnecessary and may even be your problem, or at least part of it. You're already reading correctly by inspecting the BytesToRead property and constructing a buffer of that size. – B L Aug 28 '13 at 13:10
  • No unfortunately i have seen the patterns and there is no bit that you can rely on for checking end of data – Combinu Aug 28 '13 at 13:11
  • i have removed readexisting and still i got nothing! :/ – Combinu Aug 28 '13 at 13:13
  • I'm not sure then, Mystic Jay. If you're sure your CRC code is doing what you want, then I would refine your approach in constructing the buffer and when to apply the check. Good luck. – B L Aug 28 '13 at 13:30
  • Ok than. Thanks for your time and help though! – Combinu Aug 28 '13 at 13:33
  • Troubleshoot. If you use some sample messages, does they return form CRC[0] and CRC[1] have the right value? If you are sure your algorithm is calculating the CRC correctly, just step through the code. Exactly where is it failing to approve the CRC. You have some places where I can foresee to be error prone. – Andyz Smith Aug 28 '13 at 15:23

2 Answers2

2

The problem is the use of ReadExisting(). It was not to be used in that manner as the buffer was being filled with useless data from the serial port. This problem was identified by @glace in the comments!

Combinu
  • 882
  • 2
  • 10
  • 31
1

You first need to establish communication with your meters through some existing MODBUS master application like MODPOLL. Then, once you have communication working and having valid replies from your device, then and only then start testing your code. This way you make sure that problem can be only in your code and nothing else.

For example, to connect to two slave devices at the same time RS485 must be used instead of RS232, and this requests different wiring and RS485 to RS232 convertor on PC side.

Having RX and TX connected in RS232 for simulation purpose is not a good idea since each MODBUS message from a master (except broadcast messages) needs a reply which is different from just message echo. Also, each MODBUS message from a master has MODBUS client address embedded in it and only single client should reply to it (MODBUS is single master multiple slaves protocol).

As for a CRC calculation, this might help for MODBUS RTU protocol (ASCII is different):

function mb_CalcCRC16(ptr: pointer to byte; ByteCount: byte): word;
var
  crc: word;
  b, i, n: byte;
begin
  crc := $FFFF;
  for i := 0 to ByteCount do
    if i = 0 then           // device id is 1st byte in message, and it is not in the buffer
      b := mb_GetMessageID; // so we have to calculate it and put it as 1st crc byte
    else
      b := ptr^;
      Inc(ptr);
    endif;
    crc := crc xor word(b);
    for n := 1 to 8 do
      if (crc and 1) = 1 then
        crc := (crc shr 1) xor $A001;
      else
        crc := crc shr 1;
      endif;
    endfor;
  endfor;
  Return(crc);
end;

function mb_CalcCRC: word; // Calculate CRC for message in mb_pdu
begin // this message can be one that is just received, or in a reply we have just composed
  Return(mb_CalcCRC16(@mb_pdu[1], mb_GetEndOfData));
end;

That's a quote from a working embedded AVR device with implemented MODBUS RTU slave protocol.

avra
  • 3,690
  • 19
  • 19
  • thanks for your help i actually have got the problem working... seems that i was reading wrong from the serial port!... but still help much appriciated!! – Combinu Aug 29 '13 at 09:29
  • @MysticJay is it possible to add your solution as an answer (you can answer your own question) and accept it? I think this would help future readers if they run in to similar issues. ( I refer to [xkcd](http://xkcd.com/979/) for reference) – default Aug 29 '13 at 12:56
  • i don't think this is c# – mrid Jul 26 '17 at 16:49