Mega2560 TX problem

[asefd]

Hi,
I am trying the library on Arduino Mega 2560. Receiving the queries works ok but writeRegisterToBuffer doesn't write the response to any Serial. I tried all the serials but none works.
I tried HoldingRegisters function ans also InputRegisters.
It does enter the function to handel the queries but no write to serial is being done.
Any idea?
Thanks

[yaacov]

Hi, thank you for the issue ...

hmm, writeRegisterToBuffer should write data into the buffer (not the serial port) ...
Are you sure you are using it like the example ?
https://github.com/yaacov/ArduinoModbusSlave/blob/master/examples/full/full.ino#L128

[asefd]

Yes, just like the example. With Arduino Uno it works as expected, but the same code on Mega2560 doesn't work. I tried with two different MEGA2560 but I have same problem. I asked a friend of mine and he tried with Arduino UNO and no problem and then he tried with Mega2560 but he has the same problem as me. It doesn't write to port. We have a RS485 board without the need of Control Pin.

Here is the code (We just changed the value of analogread to 221 just for testing, and it works ok on UNO);

#include <ModbusSlave.h>
// explicitly set stream to use the Serial serialport
Modbus slave(Serial2, 1); // stream = Serial, slave id = 1, rs485 control-pin = 8
void setup() {
    // register handler functions
    slave.cbVector[CB_READ_INPUT_REGISTERS] = ReadAnalogIn;
//   slave.cbVector[CB_READ_HOLDING_REGISTERS] = readMemory;
// start slave at baud 9600 on Serial
    Serial2.begin( 9600 ); // baud = 9600
    slave.begin( 9600 );
    Serial.begin( 9600 );
}
void loop() {
    // listen for modbus commands con serial port
    slave.poll();
}
// Handel Read Input Registers (FC=04)
uint8_t ReadAnalogIn(uint8_t fc, uint16_t address, uint16_t length) {
    // write registers into the answer buffer
    Serial.println(fc);
    for (int i = 0; i < length; i++) {
      //slave.writeRegisterToBuffer(i, analogRead(address + i));
      slave.writeRegisterToBuffer(i,221);
    }
    return STATUS_OK;
}

[yaacov]

With Arduino Uno it works as expected, but the same code on Mega2560 doesn't work.

Nice, can you debug the writeRegisterToBuffer and fix it for Mega2560 ?

[asefd]

Sure, can you tell me how to do debug it? Thank you

[yaacov]

Sure, can you tell me how to do debug it? Thank you

👍 I think that the easiest way will be to listen on the serial bus while adding debug data.
you can also print debug info using ghe second Serial port.

[asefd]

I am not sure if this helps. I used also a Debug library to print the value of slave.writeRegisterToBuffer

Entering function ReadAnalogIn
Function : 4 / Address : 1 / Length : 1
For i value : 0
DEBUGVAL : slave.writeRegisterToBuffer(i,221) = 0
Line after writeRegisterToBuffer
Line before return STATUS_OK

[yaacov]

a - did you try to look what happen inside the writeRegisterToBuffer ?
https://github.com/yaacov/ArduinoModbusSlave/blob/master/src/ModbusSlave.cpp#L446
b - you can also try to compare what happen in an UNO vs MEGA ?

[ysmilda]

Also, what version of the library are you using?

[asefd]

@Proace I am using the latest one.
@yaacov for the moment I don't have the UNO near me but I will try to ask my friend to see what happens. I did check the code but I will dig more into it and debug everything I can so I can post more information.

[ysmilda]

Try printing out the content of the _responseBuffer to better analyse where the data entrance stops. If the 221 value isn't entered into the buffer that will show up. Otherwise the problem probably exist with the serial, would it be possible to try Serial1 or Serial3?

[asefd]

I tried with every Serial but no success.
So, I did this...
On the main sketch I did Serial.begin(9600) and then on the ModbusSlave.cpp I do Serial.print....
I get into function writeRegisterToBuffer and everything goes well.
When I arrive at the function writeResponse, line 821 I never get to Preparing, line 845.

if (_responseBufferWriteIndex == 0 && _responseBufferLength >= MODBUS_FRAME_SIZE)
    {
        // Start the writing.
        _isResponseBufferWriting = true;
		Serial.println("start writing"); **<-- This is printed on serial output**
    }
	if(_isResponseBufferWriting || !isBroadcast()){
		Serial.println("isResponseBufferWritin || not Broadcast"); **<-- This is printed on serial output**
	}
 if (!_isResponseBufferWriting || isBroadcast())
        ; **<-- Can you explain me the purpose of this on line 836?**
    {
        _isResponseBufferWriting = false;
        _responseBufferWriteIndex = 0;
        _responseBufferLength = 0;
		Serial.println("not writing?"); **<---- This one TOO??**
        return 0;
    }

[yaacov]

Try printing out the content of the _responseBuffer

As @Proace commented, can you printout _responseBuffer . _responseBufferWriteIndex . responseBufferLength . _isResponseBufferWriting . ... anything you think can help ?

[asefd]

Ok. Can you please tell me under which function should I print the responseBuffer?
Under WriteRegisterToBuffer I have 142000 and under Poll I have 140000 and under writeResponse I have 1420221.
Sorry guys, not much experience on Arduino... or C++

[yaacov]

line 836 looks like a bug ... what happen if you remove it ?

https://github.com/yaacov/ArduinoModbusSlave/blame/master/src/ModbusSlave.cpp#L836

[ysmilda]

The weird thing being that it does work on the UNO. Following the logic of L836 the program should never send anything, which is indeed a bug.

[yaacov]

Following the logic of L836 the program should never send anything,

what happen if you remove it ?

[asefd]

I commented it and nothing is shown anymore on the Serial output... weird huh?

[yaacov]

a - L836 is a bug ( typo ... )
b - "a" may not be connected to the MEGA not working ... :-)

[ysmilda]

https://github.com/ProAce/ArduinoModbusSlave/tree/asefd
I added some Serial debugging to this branch. Could you set the Serial port to 115200 baudrate and comment the response?

[asefd]

@Proace tried your code but didn't work. Nothing shows on serial.
I am having a successful write for now as I did comment some things:

    if (!_isResponseBufferWriting && isBroadcast());
    {
        //_isResponseBufferWriting = false;
        //_responseBufferWriteIndex = 0;
        //_responseBufferLength = 0;
		Serial.println("not writing?");
        //return 0;
    }

If I don't uncomment these I have to put the famous ";" on the line 836 to have some serial output.
As I commented these lines I deleted also the ";" and it does write 221 to serial port and I receive it well on the other side.

Narrowing more....
I deleted isbroadcast() on line 835. When I serial debug this function which begins on line 289
Serial.println(Modbus::readUnitAddress()); I have double line. One reads address 1 and the other line 255 on the same time the two lines....
If I don't check if it's broadcast address then I'm fine writing the response.
@yaacov On the other side I am using your modbus-serial code on nodejs. I'm pretty sure it is set up as it should for acting as master because I use it very often and never had problem with.

[yaacov]

@asefd in the snipt above I see the extra ; after the if ...

 if (!_isResponseBufferWriting && isBroadcast()) >> ; <<
    {
        //_isResponseBufferWriting = false;

do you have it in the code you are using ?

[asefd]

@yaacov no, right now I have this code that is working:

if (!_isResponseBufferWriting)
    {
        _isResponseBufferWriting = false;
        _responseBufferWriteIndex = 0;
        _responseBufferLength = 0;
		//Serial.println("not writing?");
        return 0;
    }

[ysmilda]

Then the isBroadcast function returns true because it detects a broadcast message. The weird thing is that that should also happen on the UNO. It's also weird that you have no output at all with my code, it should at least print which function you entered.

[yaacov]

@yaacov no, right now I have this code that is working:

👍 thanks !

The weird thing is that that should also happen on the UNO.

@Proace , we don't know, AFAIU the UNO test was done by a third party thay may have used slightly different version, @asefd please correct me if I'm wrong here.

[asefd]

@Proace I will test later again and see what I can change to make it work.
@yaacov that's true. I don't have the same hardware in here to re-test the same thing and have the result because of the situation right now we're working from home...
I will dig more and see what I can find and keep you posted.
In the meantime the code is running since last night and sending the value without any problem since I removed isBroadcast . I just have to find out why this function is receiving 255 and the unit address in the same time...

[asefd]

@Proace I used your forked library. For it to work on the function Modbus::isBroadcast() line 289, I have to add a line like for ex: Serial.println(Modbus::readUnitAddress()); and it works. Nothing else changed on the code. If I don't add a line Serial.print nothing works, no output to Serial Monitor and no response written...
@yaacov same thing to your original code too.
Here is the output @Proace :

255
1
Func writeRegisterToBuffer.
598
Func writeResponse
Start writing the response
1
Start writing
Message: 
1 4 2 2 86 0 0 CRC: 28216
Length: 7
Func writeResponse
1
Start writing
Length: 0
Transmission ended

The 255 and 1 at the beggining is the Serial.println(Modbus::readUnitAddress()); that I added.
1 is the slave ID that I gave.

[yaacov]

@asefd Thanks !

Can you check if this fix the problem ?

bool Modbus::isBroadcast()
{
   uint8_t unitAddress = Modbus::readUnitAddress();

    return unitAddress == MODBUS_BROADCAST_ADDRESS;
}

[asefd]

@yaacov I just tried but it doesn't

[yaacov]

sorry to nag ... can you try ?

bool Modbus::isBroadcast()
{
   return (
       _requestBufferLength >= MODBUS_FRAME_SIZE &&
       _requestBuffer[MODBUS_ADDRESS_INDEX] == MODBUS_BROADCAST_ADDRESS);
}

[asefd]

@yaacov tried it but still not working...

[yaacov]

@yaacov tried it but still not working...

hmm ... , so after removing the ; from L836 and simplifying L289 the bug persist ...

@asefd @Proace thanks for debugging this 💚 , I do not have MEGA or UNO to check it, so my help is limited to looking at the code ...

add a line like for ex: Serial.println(Modbus::readUnitAddress()); and it works.

@asefd , can you check if you can find fix ( that does not printout debug data :-) ) ?
It sound to me like a compiler optimization bug, so moving things around may solve it (e.g. replacing the function call with an inline ... ) \o/

[asefd]

@yaacov sure I will try to find a fix. For the moment I only have the MEGA to test but as soon as I find something I can ask maybe a friend of mine to test on other devices.

[asefd]

@yaacov at line 635, do we really need to check if broadcast? If I bypass this, all good.
The problem I think is that the function isbroadcast (or readUnitAddress) at the beginning of a request is called twice at the same time. If at the line 635 I comment it, it is called once and no problem then. I tried to add a second function callling it broadcast2 and also readunitaddress2 only for testing but doesn't work. In line check doesn't work either, unless I put number 1 instead readunitaddress when I do inline check but that's not a solution.
I'm still searching and trying...

[cass3825]

solution is removing random ; found in the following routine (shown commented out, at the bottom):

uint16_t Modbus::writeResponse()
{
/**
* Validate
*/

// Check if there is a response created and that this is the first time it is written.
if (_responseBufferWriteIndex == 0 && _responseBufferLength >= MODBUS_FRAME_SIZE)
{
    // Start the writing.
    _isResponseBufferWriting = true;
}

// If we are not writing or the address is the broadcast address, cleanup and return.
if (!_isResponseBufferWriting || isBroadcast())

// ;

[yaacov]

@cass3825 Thanks, did you check on a Mega ?

Yes, we know about this bug: #55 need to fixed !

AFAIU @asefd 's problem is not related ... see #54 (comment) :-(

p.s.
Can you make a pull request fixing #55 ?

[cass3825]

Yes, removing the ; allowed the library to run successfully on a Mega2560.  I will try to make a pull request if i can remember how. Thanks for this library! On Monday, April 13, 2020, 02:02:51 AM CDT, Yaacov Zamir <notifications@github.com> wrote: @cass3825 Thanks, did you check on a Mega ? Yes, we know about this bug: #55 need to fixed ! AFAIU @asefd 's problem is not related ... see #54 (comment) :-( p.s. Can you make a pull request fixing #55 ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[falahati]

The first parameter of the callback is the slave id of the request. What is it? If you are not sending broadcast (slave id == 0) then it should be a positive number < 255. This helps to know if the problem is before the callback execution of after it.

BTW, make sure that you are not sending a broadcast message in the first place.