8000 Fix xiaomi binary_sensor warning by Danielhiversen · Pull Request #18280 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix xiaomi binary_sensor warning #18280

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 9, 2018
Merged

Fix xiaomi binary_sensor warning #18280

merged 2 commits into from
Nov 9, 2018

Conversation

Danielhiversen
Copy link
Member

Description:

Fix xiaomi binary_sensor warning
@arsaboo

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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

  • New dependencies have been added to the REQUIREMENTS variable ([example][ex-requir]).
  • New dependencies are only imported inside functions that use them ([example][ex-import]).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

@Danielhiversen Danielhiversen requested a review from syssi as a code owner November 6, 2018 21:24
@ghost ghost assigned Danielhiversen Nov 6, 2018
@ghost ghost added the in progress label Nov 6, 2018
@arsaboo
Copy link
Contributor
arsaboo commented Nov 6, 2018

Also fixes #17408

amelchio
amelchio reviewed Nov 6, 2018
View reviewed changes
Copy link
Contributor
@amelchio amelchio left a comment

Choose a reason for hiding this comment

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

This will fire the None in the event. Just checking that this is intended :)

@arsaboo
Copy link
Contributor
arsaboo commented Nov 6, 2018 •
edited
Loading

Indeed, that is not the intent. Infact, I was wondering why not use something like

        if value in ('vibrate', 'tilt', 'free_fall'):
            self.hass.bus.fire('xiaomi_aqara.movement', {
                'entity_id': self.entity_id,
                'movement_type': value
            })
            self._last_action = value
            return True
        else:
            return False

Since the other events are useless for us anyways.

@Danielhiversen
Copy link
Member Author

Ok, I see.
So None is never a real movement?

@arsaboo
Copy link
Contributor
arsaboo commented Nov 7, 2018

Yes, None is not a real movement and should be ignored

@shishos
Copy link
shishos commented Nov 7, 2018

Yes, None is not a real movement and should be ignored

If the sensor is being used to work in an environment where constant vibration is the desired and "normal" state. Wouldn't one want to know once vibration has paused and the sensor is now sampling no movement - "none"?

@arsaboo
Copy link
Contributor
arsaboo commented Nov 8, 2018

@shishos The state reverts to off after some time (which is how it works for other binary sensors).

Although, in my testing, the state is always off even though there was some movement.

image

@arsaboo
Copy link
Contributor
arsaboo commented Nov 8, 2018

Tested the code and it eliminates the noise in the logs.

@amelchio amelchio added this to the 0.82 milestone Nov 9, 2018
@amelchio amelchio merged commit c571637 into dev Nov 9, 2018
@ghost ghost removed the in progress label Nov 9, 2018
balloob pushed a commit that referenced this pull request Nov 9, 2018
@Danielhiversen @balloob
Fix xiaomi binary_sensor warning (#18280)
8ab2f66 
* Fix xiaomi binary_sensor warning

* Fix xiaomi binary_sensor warning
@balloob balloob mentioned this pull request Nov 9, 2018
Merged
@Danielhiversen Danielhiversen deleted the xiaomi-binary_sensor branch November 9, 2018 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amelchio amelchio amelchio approved these changes

@syssi syssi Awaiting requested review from syssi

Assignees

@Danielhiversen Danielhiversen

Labels
cherry-picked cla-signed integration: xiaomi_aqara small-pr PRs with less than 30 lines.
Projects
None yet
Milestone
0.82
Development

Successfully merging this pull request may close these issues.
6 participants
@Danielhiversen @arsaboo @shishos @amelchio @balloob @homeassistant
0