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

Add workaround for missing cleaning time in roomba #51163

Merged
merged 7 commits into from Jun 6, 2021
Merged

Add workaround for missing cleaning time in roomba #51163

merged 7 commits into from Jun 6, 2021

Conversation

drinfernoo
Copy link
Contributor

@drinfernoo drinfernoo commented May 27, 2021

Proposed change

This adds a simple "workaround" to calculate cleaning_time for robots which no longer report it in their state. This seems to be a due to to a recent change in the response from iRobot's firmware, in which the robot no longer reports "cleaning time" or "cleaned area".

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)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • I have followed the development checklist
  • 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:

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.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@project-bot project-bot bot added this to Needs review in Dev May 27, 2021
@drinfernoo drinfernoo changed the title 🩹 - Add workaround for missing cleaning_time Add workaround for missing cleaning_time May 27, 2021
@probot-home-assistant
Copy link

Hey there @pschmitt, @cyr-ius, @shenxn, mind taking a look at this pull request as its been labeled with an integration (roomba) you are listed as a codeowner for? Thanks!
(message by CodeOwnersMention)

@homeassistant
Copy link
Contributor

Hi @GitHub-Action,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@MartinHjelmare MartinHjelmare changed the title Add workaround for missing cleaning_time Add workaround for missing cleaning time in roomba Jun 4, 2021
Copy link
Contributor Author

@drinfernoo drinfernoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

homeassistant/components/broadlink/heartbeat.py Outdated Show resolved Hide resolved
@drinfernoo
Copy link
Contributor Author

I have been regularly rebasing onto dev, to make sure that everything stays right... but should I squash these commits?

@MartinHjelmare
Copy link
Member

No, please don't squash. We'll squash when merging. You can even just commit the review suggestions directly with github to make review even easier.

Dev automation moved this from Needs review to Reviewer approved Jun 5, 2021
Copy link
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@MartinHjelmare
Copy link
Member

Please sort the imports with isort.

@drinfernoo
Copy link
Contributor Author

Done 👍 Not sure how those got out of sorts... probably have been that way for a long time 😅

@MartinHjelmare MartinHjelmare merged commit 5bbf0ca into home-assistant:dev Jun 6, 2021
Dev automation moved this from Reviewer approved to Done Jun 6, 2021
@drinfernoo
Copy link
Contributor Author

Thank you!

@bdraco
Copy link
Member

bdraco commented Jun 6, 2021

Looks like this caused a regression

2021-06-06 17:14:50 ERROR (MainThread) [homeassistant.config_entries] Error setting up entry 89 UpstairRoomba for vacuum
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 293, in async_setup
    result = await component.async_setup_entry(hass, self)  # type: ignore
  File "/usr/src/homeassistant/homeassistant/components/vacuum/__init__.py", line 127, in async_setup_entry
    return await hass.data[DOMAIN].async_setup_entry(entry)
  File "/usr/src/homeassistant/homeassistant/helpers/entity_component.py", line 149, in async_setup_entry
    platform = await async_prepare_setup_platform(
  File "/usr/src/homeassistant/homeassistant/setup.py", line 328, in async_prepare_setup_platform
    platform = integration.get_platform(domain)
  File "/usr/src/homeassistant/homeassistant/loader.py", line 498, in get_platform
    cache[full_name] = self._import_platform(platform_name)
  File "/usr/src/homeassistant/homeassistant/loader.py", line 503, in _import_platform
    return importlib.import_module(f"{self.pkg_path}.{platform_name}")
  File "/usr/local/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/src/homeassistant/homeassistant/components/roomba/vacuum.py", line 3, in <module>
    from .braava import BraavaJet
  File "/usr/src/homeassistant/homeassistant/components/roomba/braava.py", line 6, in <module>
    from .irobot_base import SUPPORT_IROBOT, IRobotVacuum
  File "/usr/src/homeassistant/homeassistant/components/roomba/irobot_base.py", line 144, in <module>
    class IRobotVacuum(IRobotEntity, StateVacuumEntity):
  File "/usr/src/homeassistant/homeassistant/components/roomba/irobot_base.py", line 219, in IRobotVacuum
    def get_cleaning_status(self, state) -> tuple[int, int]:
TypeError: 'type' object is not subscriptable

@bdraco
Copy link
Member

bdraco commented Jun 6, 2021

Fixed in #51554

@drinfernoo
Copy link
Contributor Author

Thanks for that @bdraco 👍 In the near future, I'm going to continue to pursue the cleaned area statistic to see if there's anything that can be done about that one as well.

@AMODLT
Copy link

AMODLT commented Jun 6, 2021

No hablo bien el inglés soy de mexico y requiero apoyo en la traducción de cómo solucionar y configurar mi dispositivo

@github-actions github-actions bot locked and limited conversation to collaborators Jun 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants