Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MQTT auto-discovery/publish broken for analog sensors attached to GPIOs lower than 10 #915

Closed
gwilford opened this issue Jan 16, 2023 · 7 comments
Labels
bug Something isn't working
Milestone

Comments

@gwilford
Copy link

Description

Attaching an analog sensor to GPIO 4 results in "unknown" values in HA

Steps to reproduce

Configure GPIO less than 10 (e.g. 4) as ADC in UI (no need to connect anything to the pin)
Check value of entity in HA - it will be "unknown"

Expected behaviour

The value in HA should be 0 (or the measured value)

The issue

Data from MQTT Explorer.
The MQTT auto-discovery config:

{
  "stat_t": "ems-esp/analogsensor_data",
  "val_tpl": "{{value_json['4'].value}}",
  "object_id": "analog_sensor_Garage ldr",
  "name": "Garage ldr",
  "uniq_id": "analogsensor_4",
  "unit_of_meas": "mV",
  "dev": { 
    "ids": ["ems-esp"]
  }
}

The MQTT publish:

{
  "04": {
    "name": "Garage ldr",
    "value": 60
  }
}

The config refers to 4 whereas the publish is for 04

This is due to use of the following function in helpers.cpp for the publish but not for the config


// for decimals 0 to 99, printed as a 2 char string
char * Helpers::smallitoa(char * result, const uint8_t value) {
    result[0] = ((value / 10) == 0) ? '0' : (value / 10) + '0';
    result[1] = (value % 10) + '0';
    result[2] = '\0';
    return result;
}

Formatting the publish on line 375 of analogsensor.cpp:

JsonObject dataSensor = doc.createNestedObject(Helpers::smallitoa(s, sensor.gpio()));

Formatting the config on line 403:

snprintf(str, sizeof(str), "{{value_json['%d'].value}}", sensor.gpio());

Using smallitoa() to format the config or removing it from the publish should fix this.

Firmware v3.4.4

@gwilford gwilford added the bug Something isn't working label Jan 16, 2023
@MichaelDvP
Copy link
Contributor

Thanks for reporting.
I think changing all to "04" looks better in formatting, but breaks HA by changing also the uniq_id. So changing the publish to "4" is better. Do you agree?

@proddy
Copy link
Contributor

proddy commented Jan 16, 2023

agree. Thanks for the detailed breakdown @gwilford

@proddy proddy added this to the v3.5.0 milestone Jan 16, 2023
@gwilford
Copy link
Author

Using "04" does look better.

It would break existing uniq_id but only for configs using low GPIOs which I don't think are working anyway. However, changing the publish from "04" to "4" might break anyone who had manually configured an MQTT subscribe in HA...

@gwilford
Copy link
Author

OK, just tested and can't seem to add a manual HA mqtt sensor with "0" prefix. This fails to parse:

mqtt:
  sensor:
    - name: "EMS-ESP GPIO4"
      state_topic: "ems-esp/analogsensor_data"
      value_template: "{{ value_json.04.value }}"

with Invalid config for [mqtt]: invalid template (TemplateSyntaxError: expected token 'end of print statement', got 'integer') for dictionary value @ data['mqtt']['sensor'][0]['value_template']. Got '{{ value_json.04.value }}'. (See ?, line ?).

whereas this parses just fine:

mqtt:
  sensor:
    - name: "EMS-ESP GPIO4"
      state_topic: "ems-esp/analogsensor_data"
      value_template: "{{ value_json.4.value }}"

In summary, I don't think changing the publish to "4" would break anyone who had tried to manually configure within HA and it may well be safer.

@MichaelDvP
Copy link
Contributor

I think this should be {{ value_json.['04'].value }} to handle correctly. Or in code:
snprintf(str, sizeof(str), "{{value_json['%02d'].value}}", sensor.gpio());

@gwilford
Copy link
Author

Sorry, spoke too soon. This does currently work if manually configured as below:

mqtt:
  sensor:
    - name: "EMS-ESP GPIO4"
      state_topic: "ems-esp/analogsensor_data"
      value_template: "{{ value_json['04'].value }}"

Either fix works for me though.

MichaelDvP added a commit to MichaelDvP/EMS-ESP32 that referenced this issue Jan 16, 2023
MichaelDvP added a commit to MichaelDvP/EMS-ESP32 that referenced this issue Jan 18, 2023
@proddy
Copy link
Contributor

proddy commented Jan 22, 2023

in dev-16

@proddy proddy closed this as completed Jan 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants