Skip to content

SMA: Optional import in schema & backoff fix #18099

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

Merged
kellerza merged 2 commits into from
Nov 4, 2018
Merged

SMA: Optional import in schema & backoff fix #18099

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f58eaf
Optional import in schema & backoff fix
kellerza Nov 1, 2018
0ab561b
backoff
kellerza Nov 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions homeassistant/components/sensor/sma.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@

def _check_sensor_schema(conf):
"""Check sensors and attributes are valid."""
import pysma
try:
import pysma
except ImportError:
return conf

valid = list(conf[CONF_CUSTOM].keys())
valid.extend([s.name for s in pysma.SENSORS])
Expand Down Expand Up @@ -73,6 +76,9 @@ async def async_setup_platform(
"""Set up SMA WebConnect sensor."""
import pysma

# Check config again during load - dependancy available
Comment thread
MartinHjelmare marked this conversation as resolved.
Outdated

@tjorim tjorim Nov 4, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's try this 'suggested change' feature 😉

Suggested change
# Check config again during load - dependancy available
# Check config again during load - dependency available

@kellerza kellerza Nov 4, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks @tjorim ! I fixed this on Friday, but could not push due to issues on github. I pushed my fixes now

@kellerza kellerza Nov 4, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This suggested change feature looks quite cool though!

config = _check_sensor_schema(config)

# Sensor_defs from the custom config
for name, prop in config[CONF_CUSTOM].items():
n_s = pysma.Sensor(name, prop['key'], prop['unit'], prop['factor'])
Expand Down Expand Up @@ -107,18 +113,24 @@ async def async_close_session(event):
hass.bus.async_listen(EVENT_HOMEASSISTANT_STOP, async_close_session)

backoff = 0
backoff_step = 0

async def async_sma(event):
"""Update all the SMA sensors."""
nonlocal backoff
nonlocal backoff, backoff_step
if backoff > 1:
backoff -= 1
return

values = await sma.read(used_sensors)
if values is None:
backoff = 10
if not values:
try:
backoff = [1, 1, 1, 6, 6, 6, 30, 30][backoff_step]

@MartinHjelmare MartinHjelmare Nov 1, 2018

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 looks ok. I would probably instead use a start value that is multiplied and cap it with min and max values. The value would represent the time until next try. Eg:
https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/netgear_lte.py#L110

But maybe we need a different approach here? You know the integration best.

@kellerza kellerza Nov 2, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let’s keep this. I will change to 1,1,1,6,30 and end with the 60

Reasoning
The current implementation ensures multiples of SCAN_INTERVAL, which feels consistent for reporting. Can likely do time based delay, but it will need to take S_I into account.

The library is fairly resilient and that why I have the many 1xS_I to start. If something goes wrong (a) might be network based or (b) not sure about the exact timeout when the user will be available(might have logged in through browser etc)

backoff_step += 1
except IndexError:
backoff = 60
return
backoff_step = 0

tasks = []
for sensor in hass_sensors:
Expand Down