jdeath / rd200v2

RadonEye RD200 Version 2 Integration for Home Assistant
MIT License
73 stars 17 forks source link

Multiple rd200 devices and peak_radon possible fixes #8

Closed jbgamez closed 1 year ago

jbgamez commented 1 year ago

Hi

I've been using this custom component for about a week or so and things seem to be working well for the most part. I've been playing with the code here and there as I ran into some issues, most appear to already have been fixed, thanks! I won't add in my fixes for those as there's no need. However, there are 2 that don't appear fixed yet. I'm not all that familiar with python, so there may be better ways/places to make these changes. I list them out here, but will try to figure out how to put in a PR if that's more helpful. These seem like very minor changes though.

Peak radon:

This one looks like once it's cached it doesn't pull the full sensor data again. Restarting homeassistant or removing/readding the integration seems to work as well.

in: rd200_ble/parser.py adding a call to await client.clear_cache() in the async def update_device function seems to make peak_radon display consistently. Though it feels like there should be a better way of fixing it.

        await client.disconnect()

        await client.clear_cache()

        return device

Multiple rd200 devices:

This seems to just be a matter of making sure the 2 devices create uniquely named sensors

in: config_flow.py fixing the spelling of identifier in async def _get_device_data makes sure the value isn't blank in other areas

# - fixed spelling
        #data.indentifer = discovery_info.advertisement.local_name
        data.identifier = discovery_info.advertisement.local_name

in: sensor.py removing the identifier from the name in class RD200Sensor, removes the repeated value in the sensor name

# - removed identifier from name
        #name = f"{rd200_device.name} {rd200_device.identifier}"
        name = f"{rd200_device.name}"

in: rd200_ble/parser.py adding the name and address fields to the device object in the async def update_device function

        if ble_device.name.startswith("FR:R2"):
            device = await self._get_radon_oldVersion(client, device)
        else:
# (2 lines) - added name / address
            device.name = ble_device.name
            device.address = ble_device.address
            device = await self._get_radon(client, device)
            device = await self._get_radon_peak(client, device)
            device = await self._get_radon_uptime(client, device)

Thanks again for putting all of this together! It has been very helpful.

jbgamez commented 1 year ago

Just noticed the readme suggestion to upgrade esphome to 2022.12.4. I'll give that a shot too once I can find some more time.

Thanks again!

jdeath commented 1 year ago

I can add/fix those suggestions, or do a PR.

No need for the clear_cache(), ESPHome 12.4 will fix that. Did this already: removing the identifier from the name in class RD200Sensor, removes the repeated value in the sensor name Already removed the extra identifier. That took me a while to find. Other things look like good changes.

jdeath commented 1 year ago

Thanks. Added most of your changes. Closed with https://github.com/jdeath/rd200v2/releases/tag/2.0

Until 12.4 comes out, you can add this in your ESPHOME config recompile/upload the firmware:

external_components:
   - source: github://pr#4322
     components: [ esp32_ble_client ]