Add RSSI sensor to Home Assistant discovery - #323
Conversation
|
I haven't really looked the Theengs Gateway discovery routine in quite a while, and I;m sure you have tested and confirmed your code, but wouldn't adding This should then also allow to add the other Config relevant properties in the same conditions. I don't have any possibility to run Gateway test code for debugging at my current location, so any code which works and is approved by @1technophile is great :) |
|
You're right - that's a cleaner design. One correction though: it's not While doing this, I noticed the properties actually get loaded twice - once into Diff illustrating the new approach: development...jcsanyi:rssi-2 One side effect worth noting: this adds one iteration to the loop, so the repeated |
|
As I haven't really looked at the discovery routine for ages and since I didn't implement the original discovery routine, and not being able to do any test/debug running of the code here att the moment, and only looking at the code I still think that including rssi in the general for k in pub_device["properties"]: loop, especially since any conditional Config implementations there would also be usable for all the other Config properties. You could even already include and test the additional all Config properties in your rssi-2 code. As stated above though, however you and @1technophile decide to integrate it will be great. |
|
Sorry @DigiH - I'm not following what you're suggesting or how (if?) it's different from my rssi-2 branch. The rssi-2 code does include I'm also not sure what you're referring to with the all Config properties - are you referring to the |
|
Sorry for any confusion. I meant that your rssi-2 branch is fine as it is, even though I am not sure where it is defined that that rssi gets discovered in a separate Config section, Didn't that used to require a different publish topic? However that is handled these days, all I meant was that all the other properties which should fall into the Config section could also be included into whatever conditions is required. |
|
oh, you mean the diagnostic section in HA? That's not a different publish topic - just an additional |
|
@1technophile - let me know if/what you want me to do about the tracker publish - I could start with a separate cleanup PR that fixes the duplicate property fetching and hoists the tracker publish out of the loop, and that would make this RSSI PR much cleaner and more focused. |
getProperties was called and parsed twice for the same model, then the loop iterated one copy while reading each property's metadata out of the other. Iterating pub_device["properties"] directly drops the second parse. With one copy left, the `if k in pub_device["properties"]` guard around the metadata lookup is trivially true, so it goes as well and the block dedents. It was never doing anything: both copies came from the same getProperties output, so the key was always present. No change to what gets published: same topics, same payloads.
publish_device_tracker was called from inside the per-property loop but doesn't depend on the loop variable, so it re-published the same retained config once per decoder property: twice for HOLYIOT, BM2 and BM6, four times for Miband. Moving the call above the loop publishes it once. It stays ahead of the per-property configs, so the order on the wire is unchanged. This keeps the intent of theengs#232, which moved the call to the top of the loop body because tracker-only devices declare a single "device" property that hits the `continue` below. Above the loop it can't be skipped at all.
RSSI is scan metadata rather than a decoded value, so no decoder declares it as a property and the discovery loop never publishes it. Inject it into the loaded properties dict so it gets discovered like any other property, with signal_strength as its device class and dBm as its unit. Sets enabled_by_default false so the entity is created but stays disabled until the user enables it per device. Discussion: theengs#322
|
I updated this PR to the new implementation, stacked it on top of the cleanup in #324, and re-ran my live tests with my local gateway and devices. |
Battery level, voltage, packet counters and beacon transmit power say something about the device rather than about what it measures, so Home Assistant should file them under Diagnostics instead of listing them beside the readings. Two lists rather than one, because the two behave differently. ha_batt_diag_properties is suppressed when the decoder flags a device with "bvpp" - battery and/or voltage are primary properties - which keeps a battery monitor's battery out of the diagnostic section. ha_diag_properties is always diagnostic. The keys cover every alias each concept has in the decoder library, not just the obvious one: batt also appears as batt_l, batt_r and batt_case on AirPods, and as batt_low and lowbatt elsewhere; charging state as charging_l, charging_r and charging_case; txpower also as tx on RuuviTag; packet also as packet_1 and packet_2 on BTHome. volt is matched by key rather than by its "voltage" name, because volt_in, volt_out and volt_batt* share that name and are the actual output of the chargers and battery protects that report them. rssi matches nothing today. It is listed for the RSSI entity proposed in theengs#323, so that change needs no follow-up here.
|
Perfect, thanks! |
3 participants
Description:
Every device already publishes
rssiin its MQTT state payload, but discovery never turns it into an entity - the per-property loop only walks decoder-declared properties, andrssiisn't one. Today the only way to get an entity for it is a hand-written MQTT sensor config per device.This adds a hard-coded rssi property to the set of properties that was loaded from the decoder, resulting in a discovery item being published for it just like the other properties. The sensor uses the
signal_strengthdevice class,dBmunit, and defaults to disabled (en: false).Follows from discussion #322.
Two additional things to note:
rssiproperty, so there's no collision with the per-property loop. If one ever did, the hard-coded one provided here would overwrite it.Tested with mypy and vale, and running against my own broker and Home Assistant install with three BLE sensors - the entities show up disabled on each device page and report dBm once enabled.
Checklist: