Skip to content

Allow multiple observations of same entity#9391

Merged
MartinHjelmare merged 3 commits into
home-assistant:devfrom
jlmcgehee21:fix/bayesian_binary_enhancements
Sep 12, 2017
Merged

Allow multiple observations of same entity#9391
MartinHjelmare merged 3 commits into
home-assistant:devfrom
jlmcgehee21:fix/bayesian_binary_enhancements

Conversation

@jlmcgehee21
Copy link
Copy Markdown
Contributor

@jlmcgehee21 jlmcgehee21 commented Sep 11, 2017

Why:

  • There may be different probabilities for multiple states of the same
    entity.

This change addresses the need by:

  • Keeping a list of observations for each entity to check on each state
    change of the given entity.
  • Adding a numeric id to each observation so that they can be
    effectively added and removed from self.current_obs.
  • Adding a test to confirm functionality.

Description:

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

Checklist:

If the code does not interact with devices:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link
Copy Markdown

@jlmcgehee21, thanks for your PR! By analyzing the history of the files in this pull request, we identified @arsaboo to be a potential reviewer.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

continuation line missing indentation or outdented

Why:

* There may be different probabilities for multiple states of the same
entity.

This change addresses the need by:

* Keeping a list of observations for each entity to check on each state
change of the given entity.
* Adding a numeric id to each observation so that they can be
effectively added and removed from `self.current_obs`.
* Adding a test to confirm functionality.
self.probability = prior

self.hass.async_add_job(self.async_update_ha_state, True)
self.hass.async_add_job(self.async_update_ha_state, True)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This entity only needs to be updated after the new probability is calculated, ie outside the loop.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this, got carried away with the indenting, and all the tests passed so I guess I didn't notice 😱

@MartinHjelmare
Copy link
Copy Markdown
Member

Remove the change to the frontend and this is good to go. 👍

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Sep 12, 2017

@MartinHjelmare It's yours :)

@MartinHjelmare MartinHjelmare merged commit 29b62f8 into home-assistant:dev Sep 12, 2017
@balloob balloob mentioned this pull request Sep 22, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Mar 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants