Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
SMA: Optional import in schema & backoff fix #18099
SMA: Optional import in schema & backoff fix #18099
Changes from 1 commit
7f58eaf
0ab561b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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 😉
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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)