HA discovery template creates invalid entries when metric name contains a period #7548
Comments
|
Hi, thanks for reporting this bug. The proposed fix brakes the compatibility with the rest of the sensors. The bug is not in the autodiscovery. The bug is in the driver of the sensor. The name of the sensor has a dot and that is invalid for sensor's name. |
|
cool, thanks! |
|
Proposed PR #7549 |
|
Nice @ascillato ! Will that also affect the discovery topic? Right now that is |
ascillato2
added
bug
|
You will need to do setoption19 0, then update the firmware, and then do setoption19 1 |
|
Yes. The change in the driver will affect all topics. That is why you need first to delete the discovery settings from your broker (by setoption19 0) |
|
Right, I was suspecting this would be the case, but I could not figure out where in the code this was done. Thanks again \o |
|
@ascillato The PR breaks backwards compability. A |
|
@Jason2866 except tasmota is sending out invalid json... |
|
@Jason2866 the problem is how home assistant parses a json. The dot makes issues in home assistant side. So we can avoid that just changing the dot in json names to an underscore |
|
I'm closing the PR. I need to test another approach. |
|
Would my original proposal not let you keep the dot, and send valid templates for names that contain one? It should not change how the parsing is done for valid entries, as both are functionally identical for valid entries. |
|
@ascillato imho the approach from @cnf is the way to go. HA has problems with, so let us |
|
Yes, need testing |
|
@ascillato the SDS is the only sensor that has this issue? |
|
@effelle The PMS 5003/7003 driver has dots in the Json too. |
|
IMO, Strictly speaking, I do not know if HomeAssistant could be expecting sensor names to form valid JavaScript variable names in other places, but with careful usage of JavaScript, this does not have to be an issue. |
Correct. |
|
@cnf would you gently test this experimental build and report if the problem is solved? Thanks. |
|
@effelle OTA upgrade with that fails, is this expected? |
|
Yes, you need either to install a minimal and then the final .bin or write it trough serial. |
|
I uploaded your tasmota_experimental, but HA still doesn't discover the PM2.5. PS: I will be nice to add the unit by default: |
|
@sgrzys , I need to see your HA mqtt logs (perhaps you need to change the log level for that) because the json test report the code as valid: And query the sensor directly under HA works too:
Not possible without addding a special case for it. As for now the discovery is using the fallback |
|
Yes, the json is correct - I can take the values from manual sensor configuration with: But the discovery list has only PM10 (never discovered PM2.5): not sure about this logs from mosquito - this is just after use setoption19 1: |
|
You need to check the logs under HA not under mosquitto, because as you confirmed the json is valid. |
|
Logs form HA with debug level - no errors, but PM2.5 not discovered: |
|
Nothing better than an issue that don't trows error to close a working day. -_- |
|
OK, logs from the new one: |
|
That's funny, if the problem was the subsensor within parenthesis (but I'm sure it is not) also the PM10 should not be generated. |
|
Please also note home-assistant/core#30940 which is an home-assistant bug. Both this, and the HA bug need to be fixed for discovery to work. I have been away, but I'll try the new image tomorrow. |
|
Ok, I'll check the proposed code on that issue tomorrow and eventually I'll push a commit. |
|
@effelle can confirm your last firmware works for my setup (patched HA code). |
|
Thanks to confirm Frank. |
|
@ascillato do we have a TAG better suiting the actual situation? |
|
Like which one? |
|
I have no idea Adrian. |
|
you can temporary edit the files yourself, my report mentioned above has instructions on what to do. |
|
I've already did that but again when we speak about MQTT discovery we speak about a service, not a Tasmota - focused thing. Works for us, but need testing for other software devs. |
ascillato2
added
on hold by dev team
|
Does on hold mean it won't get merged until the HA people fix their side? It would be useful if the tasmota side works, at least. |
|
Will be merged on next update ASAP. |
|
@cnf Frank, |
ascillato2
added
fixed
PROBLEM DESCRIPTION
On certain peripherals (in my case SDS011), the metric name contains a period. With the home-assistant auto discovery, this ends up with invalid json as the template:
,"val_tpl":"{{value_json['SDS0X1'].PM2.5}}"(can't have a dot in the value name, because that is the separator)The problem is, I think here:
Tasmota/tasmota/xdrv_12_home_assistant.ino
Line 142 in bacc9e4
Proposed fix
Having this set to
",\"val_tpl\":\"{{value_json['%s']['%s']}}\"";instead seems to fix the problem.REQUESTED INFORMATION
Make sure your have performed every step and checked the applicable boxes before submitting your issue. Thank you!
Backlog Template; Module; GPIO 255:Backlog Rule1; Rule2; Rule3:Status 0:(Please use
weblog 4for more debug information)TO REPRODUCE
Use a peripheral like SDS011 with
SetOption19 1EXPECTED BEHAVIOUR
valid JSON
SCREENSHOTS
not applicable
ADDITIONAL CONTEXT
MQTT Discovery topic:
homeassistant/sensor/B29DBF_SDS0X1_PM2.5/config{"name":"tester SDS0X1 PM2.5","stat_t":"~SENSOR","avty_t":"~LWT","frc_upd":true,"pl_avail":"Online","pl_not_avail":"Offline","uniq_id":"B29DBF_SDS0X1_PM2.5","device":{"identifiers":["B29DBF"],"connections":[["mac","x:x:x:x:x:x"]]},"~":"tester/tele/","unit_of_meas":" ","val_tpl":"{{value_json['SDS0X1'].PM2.5}}"}As a sidenote, there is also a bug in home-assistant that stops discovery: home-assistant/core#30940
(Please, remember to close the issue when the problem has been addressed)
The text was updated successfully, but these errors were encountered: