Possible values for command in mb_param_request_t type were not defined or included. (IDFGH-9305)

[mrcamilletti]

I found out that were defined in modbus/include/mbproto.h but is not accesible. I think that list should be in esp_modbus_master.h or esp_modbus_common.h where is.

https://github.com/espressif/esp-modbus/blob/3f6537fc940c932884db43c9d62608e982461a70/freemodbus/common/include/esp_modbus_master.h#L102

[alisitsyn]

@mrcamilletti,

I can confirm this issue as minor. The Modbus controller code was designed to be isolated from freemodbus and mbc_master_send_request is designed to work with custom requests that may be not supported by standard modbus. This is not good idea to move these defines to esp_common.h but agree this can be useful there. Propose the change as below:

mbproto.h :

typedef enum
{
    MB_FUNC_NONE =                        (  0 ),
    MB_FUNC_READ_COILS =                  (  1 ),
    MB_FUNC_READ_DISCRETE_INPUTS =        (  2 ),
    MB_FUNC_WRITE_SINGLE_COIL =           (  5 ),
    MB_FUNC_WRITE_MULTIPLE_COILS =        ( 15 ),
    MB_FUNC_READ_HOLDING_REGISTER =       (  3 ),
    MB_FUNC_READ_INPUT_REGISTER =         (  4 ),
    MB_FUNC_WRITE_REGISTER =              (  6 ),
    MB_FUNC_WRITE_MULTIPLE_REGISTERS =    ( 16 ),
    MB_FUNC_READWRITE_MULTIPLE_REGISTERS =( 23 ),
    MB_FUNC_DIAG_READ_EXCEPTION =         (  7 ),
    MB_FUNC_DIAG_DIAGNOSTIC =             (  8 ),
    MB_FUNC_DIAG_GET_COM_EVENT_CNT =      ( 11 ),
    MB_FUNC_DIAG_GET_COM_EVENT_LOG =      ( 12 ),
    MB_FUNC_OTHER_REPORT_SLAVEID =        ( 17 ),
    MB_FUNC_ERROR =                       ( 128u )
} eMBCommands;

esp_modbus_master.h

/**
 * @brief Modbus supported commands to share with the mbc_master_send_request()
 */
typedef eMBCommands mb_function_codes_t;

/**
 * @brief Modbus register request type structure
 */
typedef struct {
    uint8_t slave_addr;                          /*!< Modbus slave address */
    mb_function_codes_t command;    /*!< Modbus command to send */ // can be left as uint8_t as well for custom commands
    uint16_t reg_start;                           /*!< Modbus start register */
    uint16_t reg_size;                            /*!< Modbus number of registers */
} mb_param_request_t;

[mrcamilletti]

Hi @alisitsyn ok understood, now I get why it was defined elsewhere. I agree with the changes to avoid redefining the commands table, or possible errors due to bad assignments. Thanks!