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 Twente Milieu integration #25129

Merged
merged 16 commits into from Jul 14, 2019

Conversation

@frenck
Copy link
Member

commented Jul 13, 2019

Description:

Adds the Twente Mileu integration

2019-07-13 21 47 06

Related issue (if applicable): n/a

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#9851

Example entry for configuration.yaml (if applicable): n/a

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

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. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

frenck added some commits Jul 13, 2019

homeassistant/components/twentemilieu/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/twentemilieu/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/twentemilieu/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/twentemilieu/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/twentemilieu/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/twentemilieu/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/twentemilieu/sensor.py Outdated Show resolved Hide resolved
"""Update Twente Milieu entity."""
next_pickup = await self._twentemilieu.next_pickup(self._waste_type)
if next_pickup is not None:
self._state = next_pickup.date().isoformat()

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Jul 13, 2019

Member

Since we have a date as state, we should set the entity device_class to 'timestamp'. It requires an iso formatted absolute utc timestamp. The frontend will be able to show this as a relative time.

This comment has been minimized.

Copy link
@frenck

frenck Jul 13, 2019

Author Member

Will try, but I don't think it has a negative effect on the purpose of this integration.
It does not have an exact time. I could set it to 00:00:00, but that is just faking it. That date will be the date the garbage is picked up, there is no time known.

I'm afraid it would result in a garbage pick up shown in the frontend in e.g., 1 hour... But that is just midnight...

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Jul 14, 2019

Member

Set the time to a constant time, the same each date.

This comment has been minimized.

Copy link
@frenck

frenck Jul 14, 2019

Author Member

Yeah, thought so. Not going there. I'm happy to adjust as soon as the system allows/shows relative time to dates and datetimes.

This will look weird ass in so many ways.

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Jul 14, 2019

Member

Here are the current options:
https://www.home-assistant.io/lovelace/entities/#format

Please make the request to the frontend team.

This comment has been minimized.

Copy link
@frenck

frenck Jul 14, 2019

Author Member

This PR has nothing to do with frontend, but backend. Backend wise this entities has a date. Not datetime. Where ever it is used, the data types should be correct and as is to start with. Adjusting the backend for the frontend sake, it just plain dead wrong. I'm not supporting that.

This comment has been minimized.

Copy link
@frenck

frenck Jul 14, 2019

Author Member

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Jul 14, 2019

Member

Sorry, I was suggesting we should ask the frontend to add this option for relative date.

I think we can keep it as absolute utc date isoformatted for now.

tests/components/twentemilieu/test_config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/twentemilieu/__init__.py Outdated Show resolved Hide resolved

frenck added some commits Jul 13, 2019

dispatcher_send -> async_dispatcher_send
Signed-off-by: Franck Nijhof <frenck@addons.community>
Removes not needed __init__
Signed-off-by: Franck Nijhof <frenck@addons.community>
Remove explicitly setting None default value on get call
Signed-off-by: Franck Nijhof <frenck@addons.community>
Correct typo in comment
Signed-off-by: Franck Nijhof <frenck@addons.community>
Clean storage for only the unloaded entry
Signed-off-by: Franck Nijhof <frenck@addons.community>
asyncio.wait on updating all integrations
Signed-off-by: Franck Nijhof <frenck@addons.community>
Use string formatting
Signed-off-by: Franck Nijhof <frenck@addons.community>
Set a more sane SCAN_INTERVAL
Signed-off-by: Franck Nijhof <frenck@addons.community>
Small refactor around services
Signed-off-by: Franck Nijhof <frenck@addons.community>

@frenck frenck requested a review from MartinHjelmare Jul 14, 2019

homeassistant/components/twentemilieu/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/twentemilieu/__init__.py Outdated Show resolved Hide resolved
"""Update Twente Milieu entity."""
next_pickup = await self._twentemilieu.next_pickup(self._waste_type)
if next_pickup is not None:
self._state = next_pickup.date().isoformat()

This comment has been minimized.

Copy link
@MartinHjelmare

MartinHjelmare Jul 14, 2019

Member

Sorry, I was suggesting we should ask the frontend to add this option for relative date.

I think we can keep it as absolute utc date isoformatted for now.

@MartinHjelmare MartinHjelmare changed the title Adds Twente Milieu integration Add Twente Milieu integration Jul 14, 2019

Extract update logic into own function
Signed-off-by: Franck Nijhof <frenck@addons.community>

@frenck frenck requested a review from MartinHjelmare Jul 14, 2019

@frenck frenck referenced this pull request Jul 14, 2019
2 of 2 tasks complete

@frenck frenck removed the docs-missing label Jul 14, 2019

@MartinHjelmare
Copy link
Member

left a comment

Great!

@MartinHjelmare MartinHjelmare merged commit 9d4b5ee into dev Jul 14, 2019

11 checks passed

CI Build #20190714.5 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pytlint) FullCheck Pytlint succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python35) Tests PyTest Python35 succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch 100% of diff hit (target 94.08%)
Details
codecov/project 94.08% (target 90%)
Details

@delete-merged-branch delete-merged-branch bot deleted the frenck-2019-0101 branch Jul 14, 2019

@frenck

This comment has been minimized.

Copy link
Member Author

commented Jul 14, 2019

Thanks for your help here @MartinHjelmare! That is greatly appreciated 🥇

@lock lock bot locked and limited conversation to collaborators Jul 15, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants
You can’t perform that action at this time.