Use entity descriptions in Roomba #102323

joostlek · 2023-10-19T13:14:22Z

Proposed change

Use entity descriptions in Roomba.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2023-10-19T13:14:30Z

Hey there @pschmitt, @cyr-ius, @shenxn, mind taking a look at this pull request as it has been labeled with an integration (roomba) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of roomba can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign roomba Removes the current integration label and assignees on the pull request, add the integration domain after the command.

# Conflicts: # homeassistant/components/roomba/irobot_base.py # homeassistant/components/roomba/sensor.py

homeassistant/components/roomba/sensor.py

Co-authored-by: J. Nick Koston <[email protected]>

homeassistant/components/roomba/sensor.py

bdraco

Tested, looks like there is an issue I didn't see in review

2023-10-19 06:34:05.245 ERROR (MainThread) [homeassistant.components.sensor] Platform roomba does not generate unique IDs. ID roomba_6BCDC6ED83A24782B24D53F0E2F1A1F1 already exists - ignoring sensor.89_upstairroomba_battery
2023-10-19 06:34:05.245 ERROR (MainThread) [homeassistant.components.sensor] Platform roomba does not generate unique IDs. ID roomba_6BCDC6ED83A24782B24D53F0E2F1A1F1 already exists - ignoring sensor.89_upstairroomba_battery
2023-10-19 06:34:05.245 ERROR (MainThread) [homeassistant.components.sensor] Platform roomba does not generate unique IDs. ID roomba_6BCDC6ED83A24782B24D53F0E2F1A1F1 already exists - ignoring sensor.89_upstairroomba_battery
2023-10-19 06:34:05.246 ERROR (MainThread) [homeassistant.components.sensor] Platform roomba does not generate unique IDs. ID roomba_6BCDC6ED83A24782B24D53F0E2F1A1F1 already exists - ignoring sensor.89_upstairroomba_battery
2023-10-19 06:34:05.246 ERROR (MainThread) [homeassistant.components.sensor] Platform roomba does not generate unique IDs. ID roomba_6BCDC6ED83A24782B24D53F0E2F1A1F1 already exists - ignoring sensor.89_upstairroomba_battery
2023-10-19 06:34:05.246 ERROR (MainThread) [homeassistant.components.sensor] Platform roomba does not generate unique IDs. ID roomba_6BCDC6ED83A24782B24D53F0E2F1A1F1 already exists - ignoring sensor.89_upstairroomba_battery
2023-10-19 06:34:05.246 ERROR (MainThread) [homeassistant.components.sensor] Platform roomba does not generate unique IDs. ID roomba_6BCDC6ED83A24782B24D53F0E2F1A1F1 already exists - ignoring sensor.89_upstairroomba_battery
2023-10-19 06:34:05.246 ERROR (MainThread) [homeassistant.components.sensor] Platform roomba does not generate unique IDs. ID roomba_6BCDC6ED83A24782B24D53F0E2F1A1F1 already exists - ignoring sensor.89_upstairroomba_battery
2023-10-19 06:34:05.294 ERROR (MainThread) [homeassistant.components.vacuum] Error adding entities for domain vacuum with platform roomba
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 507, in async_add_entities
    await asyncio.gather(*tasks)
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 752, in _async_add_entity
    await entity.add_to_platform_finish()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1023, in add_to_platform_finish
    self.async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 743, in async_write_ha_state
    self._async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 843, in _async_write_ha_state
    state, attr = self._async_generate_attributes()
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 786, in _async_generate_attributes
    attr.update(self.state_attributes or {})
                ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/vacuum/__init__.py", line 281, in state_attributes
    data[ATTR_BATTERY_LEVEL] = self.battery_level
                               ^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/roomba/irobot_base.py", line 167, in battery_level
    return self._battery_level
           ^^^^^^^^^^^^^^^^^^^
AttributeError: 'RoombaVacuumCarpetBoost' object has no attribute '_battery_level'
2023-10-19 06:34:05.299 ERROR (MainThread) [homeassistant.components.vacuum] Error while setting up roomba platform for vacuum
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 367, in _async_setup_platform
    await asyncio.gather(*pending)
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 507, in async_add_entities
    await asyncio.gather(*tasks)
  File "/usr/src/homeassistant/homeassistant/helpers/entity_platform.py", line 752, in _async_add_entity
    await entity.add_to_platform_finish()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 1023, in add_to_platform_finish
    self.async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 743, in async_write_ha_state
    self._async_write_ha_state()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 843, in _async_write_ha_state
    state, attr = self._async_generate_attributes()
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 786, in _async_generate_attributes
    attr.update(self.state_attributes or {})
                ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/vacuum/__init__.py", line 281, in state_attributes
    data[ATTR_BATTERY_LEVEL] = self.battery_level
                               ^^^^^^^^^^^^^^^^^^
  File "/usr/src/homeassistant/homeassistant/components/roomba/irobot_base.py", line 167, in battery_level
    return self._battery_level
           ^^^^^^^^^^^^^^^^^^^
AttributeError: 'RoombaVacuumCarpetBoost' object has no attribute '_battery_level'

home-assistant · 2023-10-19T16:36:39Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

bdraco · 2023-10-19T16:38:04Z

Oh


    @property
    def unique_id(self):
        """Return the uniqueid of the vacuum cleaner."""
        return self.robot_unique_id

The property is overwritten in the base class

bdraco · 2023-10-19T16:38:36Z

and it looks like _battery_level needs to still be set

joostlek · 2023-10-19T16:39:42Z

oh right, I made it public

joostlek · 2023-10-19T17:05:38Z

Heh, I tested that other PR. That seamed to work. This one suddenly doesn't work.

joostlek · 2023-10-19T17:07:26Z

ooooh, I know whats going on

joostlek · 2023-10-19T17:23:07Z

Tested locally, works

bdraco · 2023-10-19T17:24:22Z

Latest push looks good

Running to grab breakfast and errands and will test when I get back

bdraco

Tested 👍

Xitee1 · 2023-10-19T18:53:21Z

I've also tested it. At least with german translation it's not working properly. It just says "Roomba" everywhere now with no way to differentiate between the mission sensors and it seems like the battery entity now is a new sensor with different entity id.

joostlek · 2023-10-19T18:55:48Z

You have to generate translations. python3 -m script.translations develop --all

If this is your production env

cd /config
curl -o- -L https://gist.githubusercontent.com/bdraco/43f8043cb04b9838383fd71353e99b18/raw/core_integration_pr | bash /dev/stdin -d roomba -p 102323

joostlek · 2023-10-19T18:56:41Z

I must say, I also had that problem with the battery, but I couldn't figure out why. I reinstalled the integration like twice and it did not show up twice anymore, so its strange

bdraco · 2023-10-19T18:59:04Z

Let me try deleting it and re-adding it to see if I can replicate the battery issue

Xitee1 · 2023-10-19T19:00:46Z

You have to generate translations. python3 -m script.translations develop --all

If this is your production env
cd /config
curl -o- -L https://gist.githubusercontent.com/bdraco/43f8043cb04b9838383fd71353e99b18/raw/core_integration_pr | bash /dev/stdin -d roomba -p 102323

Thanks, I didn't know that. Now the translations are working properly.

bdraco · 2023-10-19T19:32:18Z

Here is what the battery looks like in the registry on dev

      {
        "aliases": [],
        "area_id": null,
        "capabilities": null,
        "config_entry_id": "3c4a5a71520c374a10a028b8bdec99c5",
        "device_class": null,
        "device_id": "86d1d524aa25c213f38d60995060a0e1",
        "disabled_by": null,
        "entity_category": "diagnostic",
        "entity_id": "sensor.89_upstairroomba_battery",
        "hidden_by": null,
        "icon": null,
        "id": "46afe441f77097e9eecfec5fcb49ce29",
        "has_entity_name": true,
        "name": null,
        "options": {
          "conversation": {
            "should_expose": false
          }
        },
        "original_device_class": "battery",
        "original_icon": null,
        "original_name": "Battery",
        "platform": "roomba",
        "supported_features": 0, 
        "translation_key": null,
        "unique_id": "battery_6BCDC6ED83A24782B24D53F0E2F1A1F1",
        "previous_unique_id": null,
        "unit_of_measurement": "%"
      },

bdraco · 2023-10-19T19:33:35Z

Its the same with this PR.

I can't replicate the issue on a test env or on production.

I will update my other production instance now

bdraco · 2023-10-19T19:34:22Z

production before this PR

bdraco · 2023-10-19T19:36:48Z

after update its fine.
confirmed the unique id didn't change

joostlek · 2023-10-19T19:37:49Z

Yea I checked my registry and got 2 entities with the same unique id. So it was kinda strange

bdraco · 2023-10-19T19:39:32Z

I'm going to clean up the typing a bit in #102350

joostlek · 2023-10-19T19:39:44Z

I saw the PR, nice one

bdraco · 2023-10-19T19:51:00Z

I've also tested it.

@Xitee1 If you plan on working on this integration long term, you should add yourself to codeowners https://github.com/home-assistant/architecture/blob/master/adr/0008-code-owners.md as we generally prioritize review for codeowners higher

Xitee1 · 2023-10-19T20:36:37Z

I've also tested it.

@Xitee1 If you plan on working on this integration long term, you should add yourself to codeowners https://github.com/home-assistant/architecture/blob/master/adr/0008-code-owners.md as we generally prioritize review for codeowners higher

Okay, I'll most likely do that.

MartinHjelmare · 2023-10-20T22:20:10Z

homeassistant/components/roomba/sensor.py

+        state_class=SensorStateClass.MEASUREMENT,
+        icon="mdi:counter",
+        entity_category=EntityCategory.DIAGNOSTIC,
+        value_fn=lambda self: self.battery_stats.get("nLithChrg")


Can we use parenthesis to add indentation so that the lambda is easier to read?

Use entity descriptions in Roomba

0c39ea2

home-assistant bot added cla-signed code-quality Hacktoberfest integration: roomba Quality Scale: No score labels Oct 19, 2023

Merge branch 'dev' into roomba_main_sensor

4da31f7

# Conflicts: # homeassistant/components/roomba/irobot_base.py # homeassistant/components/roomba/sensor.py

joostlek marked this pull request as ready for review October 19, 2023 13:52

joostlek mentioned this pull request Oct 19, 2023

Remove update_before_add from roomba #102337

Merged

20 tasks

bdraco approved these changes Oct 19, 2023

View reviewed changes

bdraco reviewed Oct 19, 2023

View reviewed changes

homeassistant/components/roomba/sensor.py Outdated Show resolved Hide resolved

Update homeassistant/components/roomba/sensor.py

719dbc0

Co-authored-by: J. Nick Koston <[email protected]>

bdraco reviewed Oct 19, 2023

View reviewed changes

homeassistant/components/roomba/sensor.py Outdated Show resolved Hide resolved

Update homeassistant/components/roomba/sensor.py

a378d07

bdraco requested changes Oct 19, 2023

View reviewed changes

home-assistant bot marked this pull request as draft October 19, 2023 16:36

Fix battery level

9fac56c

joostlek added 2 commits October 19, 2023 19:08

Fix unique_id

b5d9f44

Fix unique_id

8d4a703

joostlek marked this pull request as ready for review October 19, 2023 17:22

home-assistant bot requested a review from bdraco October 19, 2023 17:22

Merge branch 'dev' into roomba_main_sensor

f660edb

bdraco approved these changes Oct 19, 2023

View reviewed changes

bdraco mentioned this pull request Oct 19, 2023

Improve typing in roomba integration #102350

Merged

20 tasks

bdraco approved these changes Oct 19, 2023

View reviewed changes

bdraco merged commit 063d74c into home-assistant:dev Oct 19, 2023

joostlek deleted the roomba_main_sensor branch October 19, 2023 19:37

github-actions bot locked and limited conversation to collaborators Oct 20, 2023

MartinHjelmare reviewed Oct 20, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use entity descriptions in Roomba #102323

Use entity descriptions in Roomba #102323

joostlek commented Oct 19, 2023

home-assistant bot commented Oct 19, 2023

bdraco left a comment

home-assistant bot commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco commented Oct 19, 2023

joostlek commented Oct 19, 2023

joostlek commented Oct 19, 2023

joostlek commented Oct 19, 2023

joostlek commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco left a comment

Xitee1 commented Oct 19, 2023 •

edited

Loading

joostlek commented Oct 19, 2023

joostlek commented Oct 19, 2023 •

edited

Loading

bdraco commented Oct 19, 2023

Xitee1 commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco commented Oct 19, 2023

joostlek commented Oct 19, 2023

bdraco commented Oct 19, 2023

joostlek commented Oct 19, 2023

bdraco commented Oct 19, 2023

Xitee1 commented Oct 19, 2023

MartinHjelmare Oct 20, 2023

Use entity descriptions in Roomba #102323

Use entity descriptions in Roomba #102323

Conversation

joostlek commented Oct 19, 2023

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Oct 19, 2023

bdraco left a comment

Choose a reason for hiding this comment

home-assistant bot commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco commented Oct 19, 2023

joostlek commented Oct 19, 2023

joostlek commented Oct 19, 2023

joostlek commented Oct 19, 2023

joostlek commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco left a comment

Choose a reason for hiding this comment

Xitee1 commented Oct 19, 2023 • edited Loading

joostlek commented Oct 19, 2023

joostlek commented Oct 19, 2023 • edited Loading

bdraco commented Oct 19, 2023

Xitee1 commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco commented Oct 19, 2023

bdraco commented Oct 19, 2023

joostlek commented Oct 19, 2023

bdraco commented Oct 19, 2023

joostlek commented Oct 19, 2023

bdraco commented Oct 19, 2023

Xitee1 commented Oct 19, 2023

MartinHjelmare Oct 20, 2023

Choose a reason for hiding this comment

Xitee1 commented Oct 19, 2023 •

edited

Loading

joostlek commented Oct 19, 2023 •

edited

Loading