Is smarmengol Modbus software serial compatible with half duplex RS485 modules?

Projects Networking, Protocols, and Devices
1

Through the troubleshooting of other issues I noticed the following items in the smarmengol Modbus library that makes me concerned RS-485 half duplex modules will not work with the library when using a software serial port (either SoftwareSerial or AltSoftSerial).

Here are the pieces that have drawn my concern:

According to sendTxBuffer, u8serno must be >3 (4) to send via the software serial port

void Modbus::sendTxBuffer()
{
    uint8_t i = 0;

    // append CRC to message
    uint16_t u16crc = calcCRC( u8BufferSize );
    au8Buffer[ u8BufferSize ] = u16crc >> 8;
    u8BufferSize++;
    au8Buffer[ u8BufferSize ] = u16crc & 0x00ff;
    u8BufferSize++;

    // set RS485 transceiver to transmit mode
    if (u8txenpin > 1)
    {
        switch( u8serno )
        {
#if defined(UBRR1H)
        case 1:
            UCSR1A=UCSR1A |(1 << TXC1);
            break;
#endif

#if defined(UBRR2H)
        case 2:
            UCSR2A=UCSR2A |(1 << TXC2);
            break;
#endif

#if defined(UBRR3H)
        case 3:
            UCSR3A=UCSR3A |(1 << TXC3);
            break;
#endif
        case 0:
        default:
            UCSR0A=UCSR0A |(1 << TXC0);
            break;
        }
        digitalWrite( u8txenpin, HIGH );
    }

    // transfer buffer to serial line
    if(u8serno<4)
        port->write( au8Buffer, u8BufferSize );
    else
        softPort->write( au8Buffer, u8BufferSize );

    // keep RS485 transceiver in transmit mode as long as sending
    if (u8txenpin > 1)
    {
        switch( u8serno )
        {
#if defined(UBRR1H)
        case 1:
            while (!(UCSR1A & (1 << TXC1)));
            break;
#endif

#if defined(UBRR2H)
        case 2:
            while (!(UCSR2A & (1 << TXC2)));
            break;
#endif

#if defined(UBRR3H)
        case 3:
            while (!(UCSR3A & (1 << TXC3)));
            break;
#endif
        case 0:
        default:
            while (!(UCSR0A & (1 << TXC0)));
            break;
        }

        // return RS485 transceiver to receive mode
        digitalWrite( u8txenpin, LOW );
    }
    if(u8serno<4)
        while(port->read() >= 0);
    else
        while(softPort->read() >= 0);

    u8BufferSize = 0;

    // set time-out for master
    u32timeOut = millis();

    // increase message counter
    u16OutCnt++;
}

/**
 * @brief
 * Full constructor for a Master/Slave through USB/RS232C/RS485
 * It needs a pin for flow control only for RS485 mode
 *
 * @param u8id   node address 0=master, 1..247=slave
 * @param u8serno  serial port used 0..3
 * @param u8txenpin pin for txen RS-485 (=0 means USB/RS232C mode)
 * @ingroup setup
 * @overload Modbus::Modbus(uint8_t u8id, uint8_t u8serno, uint8_t u8txenpin)
 * @overload Modbus::Modbus(uint8_t u8id)
 * @overload Modbus::Modbus()
 */
Modbus::Modbus(uint8_t u8id, uint8_t u8serno, uint8_t u8txenpin)
{
    init(u8id, u8serno, u8txenpin);
}

Modbus::Modbus(uint8_t u8id, uint8_t u8serno)
{
    init(u8id, u8serno, 0);
}

Modbus::Modbus(uint8_t u8id)
{
    init(u8id);
}

The three constructor functions call these to init functions:

void Modbus::init(uint8_t u8id, uint8_t u8serno, uint8_t u8txenpin)
{
    this->u8id = u8id;
    this->u8serno = (u8serno > 3) ? 0 : u8serno;
    this->u8txenpin = u8txenpin;
    this->u16timeOut = 1000;
}

void Modbus::init(uint8_t u8id)
{
    this->u8id = u8id;
    this->u8serno = 4;
    this->u8txenpin = 0;
    this->u16timeOut = 1000;
}

To make u8serno=4, it can only be constructed using Modbus master_slave(ID) but in doing so sets the u8txenpin to 0.

If u8txenpin needs to >1 then the constructor Modbus master_slave(ID, serial_num, 10) needs to be used. But if serial_num >3, it is set to 0 (note the line: this->u8serno = (u8serno > 3) ? 0 : u8serno; ).

Am I over analyzing this and completely off base or are more modifications to the library in my future?

2

are more modifications to the library in my future?

I think so. This doesn't make sense:

        case 0:
        default:
            UCSR0A=UCSR0A |(1 << TXC0);
            break;

That looks like code for case 0, not case 4 (default). When it's a software serial library, there is no "transmit complete" flag, and there is no "control and status register".

Unfortunately, AltSoftSerial and NeoSWSerial differ in how they handle transmission: AltSoftSerial has a transmit buffer that empties in the background, while NeoSWSerial blocks until the characters are transmitted. Detecting "transmission complete" will be different for these two libraries.

AltSoftSerial is much more efficient and reliable than NeoSWSerial, so I would recommend it any day of the week. You may have to use time to "know" that the AltSoftSerial transmit buffer is empty... unless you want to modify AltSoftSerial, too. :wink:

Hey, I already did that with NeoICSerial to add the attachInterrupt feature for RX chars. I could add a "TX complete" callback, too.

3

-dev:
AltSoftSerial is much more efficient and reliable than NeoSWSerial, so I would recommend it any day of the week. You may have to use time to "know" that the AltSoftSerial transmit buffer is empty... unless you want to modify AltSoftSerial, too. :wink:

I'll get back to that.

-dev:
I think so. This doesn't make sense:

        case 0:

default:
            UCSR0A=UCSR0A |(1 << TXC0);
            break;



That looks like code for case 0, not case 4 (default). When it's a software serial library, there is no "transmit complete" flag, and there is no "control and status register".

Unfortunately, AltSoftSerial and NeoSWSerial differ in how they handle transmission: AltSoftSerial has a transmit buffer that empties in the background, while NeoSWSerial blocks until the characters are transmitted. Detecting "transmission complete" will be different for these two libraries.

I hadn't noticed that.

I was looking at

if(u8serno<4)
        port->write( au8Buffer, u8BufferSize );
    else
        softPort->write( au8Buffer, u8BufferSize );

That requires u8serno to be >= 4 to use the software serial port. But to make it 4 then txenpin is set to 0 per

void Modbus::init(uint8_t u8id)
{
    this->u8id = u8id;
    this->u8serno = 4;
    this->u8txenpin = 0;
    this->u16timeOut = 1000;
}

If I try to use

void Modbus::init(uint8_t u8id, uint8_t u8serno, uint8_t u8txenpin)
{
    this->u8id = u8id;
    this->u8serno = (u8serno > 3) ? 0 : u8serno;
    this->u8txenpin = u8txenpin;
    this->u16timeOut = 1000;
}

and set u8serno to 4, then the if statement catches is and forces it back to 0 (hardware Serial).

What would be the point of that logic? Incomplete implementation?

4

What would be the point of that logic? Incomplete implementation?

Seems like it. Stepping on Serial's registers is not a Good Thing.

If you use AltSoftSerial, you could call flushOutput. It will wait for the last stop bit to be shifted out. I don't know if the ModBus library has an ISR for TXComplete, so if you don't want to block at flushOutput, you'll have to modify AltSoftSerial (or use the one I modified, NeoICSerial). I'm adding the TxComplete callback now...

5

Wonderful. :sob:

Looks like there is a bunch more integration left to be done in smarmengol Modbus library to incorporate the software serial, or at least to incorporate AltSoftSerial.

First I'm going to fix the constructors to allow the software serial port to work with the half duplex RS-485 boards.

I can't decide between just changing

this->u8serno = (u8serno > 3) ? 0 : u8serno; to this->u8serno = u8serno;

or also

Forcing the full constructor to always be used (Modbus::Modbus(uint8_t u8id, uint8_t u8serno, uint8_t u8txenpin) )

I leaning towards plan B as it reduces and simplifies the library.

6

Ok, I just checked in the new version of NeoICSerial with attachTxCompletedInterrupt.

FWIW, if I were modifying the library, I would fix the whole HW vs. SW serial port goofiness.The library should just hold on to a Stream * for the port pointer. I assume that port is declared like this:

    HardwareSerial *port;

If you change it to this:

    Stream *port;

Then it could hold on to any of the hardware or software serial ports. I guess I would have expected constructors that take a few params and a Stream *.

Modbus::Modbus(uint8_t u8id, Stream *stream, uint8_t u8txenpin)
{
    init(u8id, stream, u8txenpin);
}

void Modbus::init(uint8_t u8id, uint8_t u8serno, uint8_t u8txenpin)
{
    this->u8id = u8id;
    this->port = stream;
    this->u8txenpin = u8txenpin;
    this->u16timeOut = 1000;
}

... and called like this:

   AltSoftSerial modbusPort;
       ...
   Modbus modbus( MODBUS_ID, &modbusPort, TX_EN_PIN );

I'm pretty sure that the rest of the library won't know the difference, as it probably just uses available, read and write (and print variants).

If it calls begin for you, why? Delete that code and just call begin before you start the modbus library. :-/

I'm not sure how the TxComplete factors into this. If there are places in the library that wait for that UART status register bit to clear, I'd say switch to using flush everywhere.

If they were defining their own TXC_ISR, you would have to expose the TXC code in the library header, and connect it from the caller (the sketch). Sorry, I just haven't taken the time to look at the library.

But you're doing the work, not me. :slight_smile:

7

-dev:
FWIW, if I were modifying the library, I would fix the whole HW vs. SW serial port goofiness.The library should just hold on to a Stream * for the port pointer. I assume that port is declared like this:

    HardwareSerial *port;

If you change it to this:

    Stream *port;

Then it could hold on to any of the hardware or software serial ports. I guess I would have expected constructors that take a few params and a Stream *.

Seems like I have seen this modification somewhere recently...@pylon...

I'm a one-change-at-a-time type person. Especially when it comes to "clean-up programming".