ModbusException: Confusing parameter names vs. Modbus spec, results in incorrect byte ordering in `toRaw()`

[david-drinn]

The MODBUS Application Protocol Specification V1.1b3 defines the exception/error message as follows:

Or in other words:

Example for an exception/error resulting from a 02 (0x02) Read Discrete Inputs request:

To start with, the ModbusException class variables are not very clear:

class ModbusException : public std::exception {
  private:
    uint8_t _slaveId;
    bool _validSlave;
    utils::MBErrorCode _errorCode;
    utils::MBFunctionCode _functionCode;

The _errorCode here in the code represents the "Exception code" in the spec, for example from this snippet, these are clearly "Exception codes" not the "Error codes":

enum MBErrorCode : uint8_t {
    // Documentation modbus errors
    IllegalFunction                    = 0x01,
    IllegalDataAddress                 = 0x02,
    IllegalDataValue                   = 0x03,
    SlaveDeviceFailure                 = 0x04,
    // snipped
};

Similarly, the class variable _functionCode in the code represents (after calculation) the "Error code" in the Modbus spec, as shown in this snippet of its type:

enum MBFunctionCode : uint8_t {
    // Reading functions
    ReadDiscreteOutputCoils          = 0x01,
    ReadDiscreteInputContacts        = 0x02,
    ReadAnalogOutputHoldingRegisters = 0x03,
    ReadAnalogInputRegisters         = 0x04,
    // snipped
};

This is fine, obfuscating out the "Error code" calculation of "Function code" of the original request, plus 0x80 (set highest bit), is nice for an API.

But when it gets converted to the raw bytes in toRaw() it gets mixed up:

std::vector<uint8_t> ModbusException::toRaw() const noexcept {
    std::vector<uint8_t> result(3);

    result[0] = _slaveId;
    result[1] = static_cast<uint8_t>(_errorCode | 0b10000000);
    result[2] = static_cast<uint8_t>(_functionCode);

    return result;
}

Here the _errorCode (which we already established is the "Exception code", despite the variable name) is getting treated as the "Function code" / "Error code", while _functionCode (which we already established is the "Function code" / "Error code") is getting treated as the "Exception code".

Instead, it should be this order and calculation:

std::vector<uint8_t> ModbusException::toRaw() const noexcept {
    std::vector<uint8_t> result(3);

    result[0] = _slaveId;
    result[1] = static_cast<uint8_t>(_functionCode | 0b10000000);
    result[2] = static_cast<uint8_t>(_errorCode);

    return result;
}

Though, again, the variable name, _errorCode clashes with the Modbus spec, making it seem wrong.