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

Fix Mi Flora median calculation #16085

Merged
merged 9 commits into from
Sep 4, 2018

Conversation

PaulAnnekov
Copy link
Contributor

@PaulAnnekov PaulAnnekov commented Aug 20, 2018

Description:

miflora library caches values for 20 minutes. miflora HA component updates values each 30 seconds. Component gets 3 value updates, calculates median and sets state. During 20 minutes we get the same values from cache which makes median calculation totally useless - all values will be the same.

  • Removed cache_value conf in favor of native scan_interval, which is now set to 1200 seconds and used for both update interval and cache timeout in the library.
  • Added initial value on component start, to show at least anything before 40+ minutes will past and median will be calculated
  • Added support for PlatformNotReady to don't delay HA startup
  • Removed ble_timeout and retries, they are not used in miflora library anymore and, actually, we don't need them

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io/pull/6163

Breaking changes:

Removed retries and timeout config parameters, they were not used for several months. Replaced cache_value config with scan_interval to fix a bug in the PR.

Checklist:

  • Update docs.
  • Deal with initial value, so user won't wait 1 hour to see first data.
  • The code change is tested and works locally.

})


def setup_platform(hass, config, add_devices, discovery_info=None):
@asyncio.coroutine

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

})


def setup_platform(hass, config, add_entities, discovery_info=None):
@asyncio.coroutine
def async_setup_platform(hass, config, async_add_entities, discovery_info=None):

Choose a reason for hiding this comment

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

line too long (80 > 79 characters)

@PaulAnnekov PaulAnnekov changed the title (WIP) Fixed Mi Flora calculated median from the same cached values Fixed Mi Flora calculated median from the same cached values Sep 2, 2018
homeassistant/components/sensor/miflora.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/miflora.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/miflora.py Outdated Show resolved Hide resolved
})


def setup_platform(hass, config, add_entities, discovery_info=None):
async def async_setup_platform(hass, config, async_add_entities,
discovery_info=None):

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

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.

Good!

Please add a paragraph about the breaking change in the PR description for the release notes.

@PaulAnnekov
Copy link
Contributor Author

@MartinHjelmare done.

homeassistant/components/sensor/miflora.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/miflora.py Outdated Show resolved Hide resolved
homeassistant/components/sensor/miflora.py Outdated Show resolved Hide resolved
@MartinHjelmare MartinHjelmare changed the title Fixed Mi Flora calculated median from the same cached values Fix Mi Flora median calculation Sep 4, 2018
@MartinHjelmare MartinHjelmare merged commit e1501c8 into home-assistant:dev Sep 4, 2018
@ghost ghost removed the in progress label Sep 4, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
* fixed median was based on 1.5 minute interval, not 1 hour

* ignore median and set state from first value, when previous state was None

* update before add, removed unused 'retries' and 'ble_timeout', check if platform ready

* added missing blank line

* fixed too long line

* using modern python 3.5 features, changed comment to be less verbose

* continuation line fix

* removed DEFAULT_SCAN_INTERVAL in favor of existing SCAN_INTERVAL
@balloob balloob mentioned this pull request Sep 17, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 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.

4 participants