espressif / esp-modbus

ESP-Modbus - the officially suppported library for Modbus protocol (serial RS485 + TCP over WiFi or Ethernet).
Apache License 2.0
85 stars 46 forks source link

When the result is of float type and the value is 0.xxx, a parsing error will occur. (IDFGH-12632) #56

Closed yel-best closed 3 months ago

yel-best commented 3 months ago

use idf v5.1.3 use modbus v1.0.13

When I collected the data, I used IEEE754 to obtain float type data with a result less than 0, for example: 0.13, 0.22. My previous issue mentioned that the data order was converted to support the CDBA format. https://github.com/espressif/esp-modbus/issues/51

MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB 999a4360 = 4360999a MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB 1eb83e05 = 3e051eb8 MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB d5673ce7 = 3ce7d567 MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB c28f3e75 = 3e75c28f

Except for the first result read above, the analysis is correct, but the following should all have values. 0.xxx but the output will be 0. Can you help me take a look?

W (124586) MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB 999a4360 = 4360999a
I (124586) app_modbus: Characteristic #0 Voltage (V) value = 0.000000 (0x224) read successful.
I (124592) app_modbus: MB readings stored: for Voltage with value: 224.000000

W (124628) MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB 1eb83e05 = 3e051eb8
I (124628) app_modbus: Characteristic #1 I (A) value = 0.000000 (0x0) read successful.
I (124634) app_modbus: MB readings stored: for Ampere with value: 0.000000

W (124668) MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB d5673ce7 = 3ce7d567
I (124668) app_modbus: Characteristic #2 P (kW) value = 0.000000 (0x0) read successful.
I (124674) app_modbus: MB readings stored: for kW with value: 0.000000

W (124708) MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB c28f3e75 = 3e75c28f
I (124708) app_modbus: Characteristic #3 ImpEp (kWh) value = 0.000000 (0x0) read successful.
I (124714) app_modbus: MB readings stored: for totalKwh with value: 0.000000

part of my code

mb_parameter_descriptor_t device_parameters[] = {
    {CID_POWER_Voltage, STR("Voltage"), STR("V"), MB_DEVICE_ADDR1, MB_PARAM_HOLDING, 0x2000, 2,
     HOLD_OFFSET(holding_data0), PARAM_TYPE_FLOAT_CDAB, 4, OPTS(0, 0, 1), PAR_PERMS_READ},
    {CID_POWER_I, STR("I"), STR("A"), MB_DEVICE_ADDR1, MB_PARAM_HOLDING, 0x2002, 2,
     HOLD_OFFSET(holding_data0), PARAM_TYPE_FLOAT_CDAB, 4, OPTS(0, 0, 1), PAR_PERMS_READ},
    {CID_POWER_P, STR("P"), STR("kW"), MB_DEVICE_ADDR1, MB_PARAM_HOLDING, 0x2004, 2,
     HOLD_OFFSET(holding_data0), PARAM_TYPE_FLOAT_CDAB, 4, OPTS(0, 0, 1), PAR_PERMS_READ},
    {CID_POWER_ImpEp, STR("ImpEp"), STR("kWh"), MB_DEVICE_ADDR1, MB_PARAM_HOLDING, 0x4000, 2,
     HOLD_OFFSET(holding_data0), PARAM_TYPE_FLOAT_CDAB, 4, OPTS(0, 0, 1), PAR_PERMS_READ},
};

err = mbc_master_get_cid_info(cid, &param_descriptor);
        if ((err != ESP_ERR_NOT_FOUND) && (param_descriptor != NULL))
        {
            void *temp_data_ptr = master_get_param_data(param_descriptor);
            assert(temp_data_ptr);
            uint8_t type = 0;

            err = mbc_master_get_parameter(cid, (char *)param_descriptor->param_key,
                                           (uint8_t *)&current_value, &type);
            if (err == ESP_OK)
            {
                *(float *)temp_data_ptr = current_value;
                if (param_descriptor->mb_param_type == MB_PARAM_HOLDING)
                {
                    ESP_LOGI(TAG, "Characteristic #%d %s (%s) value = %lf (0x%" PRIu32 ") read successful.",
                             param_descriptor->cid,
                             (char *)param_descriptor->param_key,
                             (char *)param_descriptor->param_units,
                             current_value,
                             *(uint32_t *)temp_data_ptr);

                    uint32_t current_value = *(uint32_t *)temp_data_ptr;

                    // Add the informative triplets to the array, ready to report/send
                    mb_readings[cid].key = param_descriptor->param_key;
                    mb_readings[cid].value = current_value;
                    mb_readings[cid].unit = param_descriptor->param_units;
                    // ESP_LOGI(TAG, "MB readings stored: for %s with value: %f\n", mb_readings[cid].key, mb_readings[cid].value);
                }
            }
        }
yel-best commented 3 months ago

image

W (5576) MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB 999a4360 = 4360999a
I (5576) app_modbus: MB readings stored: for Voltage with value: 224.000000

I (5581) app_modbus: MB readings stored: for Voltage with value: 224.000000
yel-best commented 3 months ago

According to previous communication, the conversion I used is as follows

   // Fix endianess for FLOAT and U32 according to your slave manual
        if (((reg_info.param_type == PARAM_TYPE_FLOAT_CDAB)) && (reg_info.param_size == 4))
        {
            float dest_val = 0;
            _XFER_4_FLOAT_CDAB(&dest_val, value_ptr);
            ESP_LOGW(TAG, "Convert PARAM_TYPE_FLOAT_CDAB %" PRIx32 " = %" PRIx32, *(uint32_t *)value_ptr, *(uint32_t *)&dest_val);
            *(uint32_t *)value_ptr = (uint32_t)dest_val;
        }

#define _XFER_4_FLOAT_CDAB(dst, src) { \
    *(uint8_t *)((uint8_t*)dst + 3) = *(uint8_t *)(src + 1); \
    *(uint8_t *)((uint8_t*)dst + 2) = *(uint8_t *)(src + 0); \
    *(uint8_t *)((uint8_t*)dst + 0) = *(uint8_t *)(src + 2); \
    *(uint8_t *)((uint8_t*)dst + 1) = *(uint8_t *)(src + 3); \
}
alisitsyn commented 3 months ago

@yel-best ,

Thanks for the issue. I will take a look to the details of your issue soon. However, I also recommend you to wait a bit to test extended types support for modbus stack. The extended types support is almost done for stack v1.0.x and v2.0.0-beta and will be merged soon. I will be able to give you update soon or the link to preliminary branch to test this. Let me know if this works for you.

yel-best commented 3 months ago

@yel-best ,

Thanks for the issue. I will take a look to the details of your issue soon. However, I also recommend you to wait a bit to test extended types support for modbus stack. The extended types support is almost done for stack v1.0.x and v2.0.0-beta and will be merged soon. I will be able to give you update soon or the link to preliminary branch to test this. Let me know if this works for you.

thanks. I wonder if the data accuracy is lost due to conversion. After I obtained the hex code of modbus, the float conversion corresponded to the CDAB format, as follows


// Get parameter data for corresponding characteristic
static esp_err_t mbc_serial_master_get_parameter(uint16_t cid, char* name,
                                                    uint8_t* value_ptr, uint8_t *type)
{
    MB_MASTER_CHECK((name != NULL),
                        ESP_ERR_INVALID_ARG, "mb incorrect descriptor.");
    MB_MASTER_CHECK((type != NULL),
                        ESP_ERR_INVALID_ARG, "type pointer is incorrect.");
    esp_err_t error = ESP_ERR_INVALID_RESPONSE;
    mb_param_request_t request ;
    mb_parameter_descriptor_t reg_info = { 0 };

    error = mbc_serial_master_set_request(name, MB_PARAM_READ, &request, &reg_info);
    if ((error == ESP_OK) && (cid == reg_info.cid)) {
        // Send request to read characteristic data
        error = mbc_serial_master_send_request(&request, value_ptr);
        if (error == ESP_OK) {
            ESP_LOGD(TAG, "%s: Good response for get cid(%u) = %s",
                                    __FUNCTION__, (unsigned)reg_info.cid, (char*)esp_err_to_name(error));
        } else {
            ESP_LOGD(TAG, "%s: Bad response to get cid(%u) = %s",
                                            __FUNCTION__, (unsigned)reg_info.cid, (char*)esp_err_to_name(error));
        }
        // Set the type of parameter found in the table
        *type = reg_info.param_type;

        if (((reg_info.param_type == PARAM_TYPE_FLOAT) || (reg_info.param_type == PARAM_TYPE_U32)) && (reg_info.param_size == 4))
        {
            // Fix endianess for FLOAT and U32 according to your slave manual
            float dest_val = 0;
            _XFER_4_FLOAT(&dest_val, value_ptr);
            ESP_LOGW(TAG, "Convert %" PRIx32 " = %" PRIx32, *(uint32_t *)value_ptr, *(uint32_t *)&dest_val);
            *(uint32_t *)value_ptr = (uint32_t)dest_val;
        }
    } else {
        ESP_LOGE(TAG, "%s: The cid(%u) not found in the data dictionary.",
                                                    __FUNCTION__, (unsigned)reg_info.cid);
        error = ESP_ERR_INVALID_ARG;
    }
    return error;
}
alisitsyn commented 3 months ago

thanks. I wonder if the data accuracy is lost due to conversion.

I think you are right here. I will check this a bit later and will let you know. Also, check your code for correct conversion.

After I obtained the hex code of modbus, the float conversion corresponded to the CDAB format, as follows

Part of documentation for new version of stack is below:

Mapping Of Complex Data Types

As per section 4.2 of Modbus specification, “MODBUS uses a big-Endian representation for addresses and data items. This means that when a numerical quantity larger than a single byte is transmitted, the most significant byte is sent first”. The biggest official structure defined by the Modbus specification is a 16-bit word register, which is 2 bytes. However, vendors sometimes group two or even four 16-bit registers together to be interpretted as 32-bit or 64-bit values, respectively. It is also possible when the Modbus vendors group many registers together for serial numbers, text strings, time/date, etc. Regardless of how the vendor intends the data to be interpreted, the Modbus protocol itself simply transfers 16-bit word registers. These values grouped from registers may use either little-endian or big-endian register order.

Each individual 16-bit register, is encoded in big-endian order (assuming the Modbus device abides by the Modbus specification). However, the 32-bit and 64-bit types naming conventions like ABCD or ABCDEFGH, does not take into account the network format byte order of frame. For example: the ABCD prefix for 32-bit values means the common Modbus mapping format and corresponds to the CDAB on network format (order in the frame).

I think this should explain on how the frames are constructed. I also have some diagrams there to explain all details.

image

image

yel-best commented 3 months ago

Does it mean that we only need to convert the FLOAT result (uint32_t )value_ptr = (uint32_t)dest_val; to the logic?

Through this method, I converted the hex code read by modbus into the correct byte order. If the converted part can be directly decoded and the IEEE754 FLOAT value is obtained, does it mean it is feasible? Although it may be a special format, the supplier may also have some strange practices

#define _XFER_4_FLOAT(dst, src)                               \
{                                                             \
    *(uint8_t *)((uint8_t *)dst + 3) = *(uint8_t *)(src + 1); \
    *(uint8_t *)((uint8_t *)dst + 2) = *(uint8_t *)(src + 0); \
    *(uint8_t *)((uint8_t *)dst + 0) = *(uint8_t *)(src + 2); \
    *(uint8_t *)((uint8_t *)dst + 1) = *(uint8_t *)(src + 3); \
}

MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB 999a4360 = 4360999a MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB 1eb83e05 = 3e051eb8 MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB d5673ce7 = 3ce7d567 MB_CONTROLLER_MASTER: Convert PARAM_TYPE_FLOAT_CDAB c28f3e75 = 3e75c28f

alisitsyn commented 3 months ago

If the converted part can be directly decoded and the IEEE754 FLOAT value is obtained, does it mean it is feasible

Yes.

Please check your code and the type of your value you return. You get your float value incorrectly after conversion to unsigned integer. You can also wait the updated library which supports all possible types.

*(uint32_t *)value_ptr = (uint32_t)dest_val;

yel-best commented 3 months ago
 if (((reg_info.param_type == PARAM_TYPE_FLOAT) || (reg_info.param_type == PARAM_TYPE_U32)) && (reg_info.param_size == 4))
        {
            // Fix endianess for FLOAT and U32 according to your slave manual
            float dest_val = 0;
            _XFER_4_FLOAT(&dest_val, value_ptr);
            ESP_LOGW(TAG, "Convert %" PRIx32 " = %" PRIx32, *(uint32_t *)value_ptr, *(uint32_t *)&dest_val);
            *(uint32_t *)value_ptr = (uint32_t)dest_val;
        }

Thank you for answering my questions despite your busy work.

The conversion I understand is this piece of code,

_XFER_4_FLOAT(&dest_val, value_ptr);
ESP_LOGW(TAG, "Convert %" PRIx32 " = %" PRIx32, *(uint32_t *)value_ptr, *(uint32_t *)&dest_val);

When I convert in this way, I can get the correct byte HEX order, so where should the problem lie? *(uint32_t *)value_ptr = (uint32_t)dest_val;

Because what I need is float, but the result converted here is uint32_t

Is my understanding correct?

 if (((reg_info.param_type == PARAM_TYPE_FLOAT) || (reg_info.param_type == PARAM_TYPE_U32)) && (reg_info.param_size == 4))
        {
            // Fix endianess for FLOAT and U32 according to your slave manual
            float dest_val = 0;
            _XFER_4_FLOAT(&dest_val, value_ptr);
            ESP_LOGW(TAG, "Convert %" PRIx32 " = %" PRIx32, *(uint32_t *)value_ptr, *(uint32_t *)&dest_val);
            *(uint32_t *)value_ptr = (uint32_t)dest_val;
        }
alisitsyn commented 3 months ago

Yes, this is what I mentioned above.

alisitsyn commented 3 months ago

@yel-best , the issue will be closed for now. Feel free to reopen. I will update here once the extended types support is merged. I would ask you to check the update with your device.

alisitsyn commented 3 months ago

@yel-best , This update which supports the extended types is merged with 8480ff7636eda25e0ebb81ef36552cdfd814f535. Please take a look to updated documentation. If it is possible please try to address your device parameters using this update. Thanks.

yel-best commented 2 months ago

@alisitsyn Wow, this is great, love it, thank you for your help and dedication, thank you again! ! ! 💘