-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
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
Add current intraday price ranking to Tibber price sensor #124595
Add current intraday price ranking to Tibber price sensor #124595
Conversation
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.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Hey there @Danielhiversen, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
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.
Please wait for this PR to be solved:
#124407
Ah yes 🙈 I actually looked at it, but missed the I'll wait for that to be merged and rebase + resubmit 👍 |
09785a6
to
0b29a21
Compare
There we go ✔️ After waiting 6 months to follow up my new "feature", I'm one day early with the PR 😆 |
And FYI. I'm getting this error when starting HA locally: 2024-08-27 12:53:36.477 ERROR (MainThread) [tibber.home] Error in rt_subscribe
Traceback (most recent call last):
File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/tibber/home.py", line 460, in _start
data = _add_extra_data(data)
^^^^^^^^^^^^^^^^^^^^^
File "/home/vscode/.local/ha-venv/lib/python3.12/site-packages/tibber/home.py", line 421, in _add_extra_data
if live_data.get("powerProduction", 0) > 0 and live_data.get("power") is None:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '>' not supported between instances of 'NoneType' and 'int' Seems like the |
If possible, please make a PR with a fix to pyTibber if there's a bug there. |
PR is here: Danielhiversen/pyTibber#303 This is unrelated to the change in this PR of course. But if you want I can make it part of this PR to bump the dependency to a new pyTibber and verify it. |
It's enough if you've tested locally with the fix in pyTibber that this PR works. |
I've tested the fix in pyTibber and it stops throwing exceptions. So that's why I'm asking if you'll build a new release of pyTibber and i could bump it here. And just to be clear. Both the bug, and fix, in pyTibber has nothing to do with |
I don't maintain pyTibber so I can't cut a new release there on my own. Daniel maintains that library. |
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.
Looks good!
Please undraft the PR when ready to merge.
Proposed change
Upgrading pyTibber dependency from
0.28.2
to0.29.0
. See the release notes for more details.It includes support for Python 3.12, Ruff upgrades, updates to documentation and a new attribute for the price sensor.
The new attribute breaks one of the calls from this integration to the pyTibber library and I'm adding support for it here.
PR for this change is here: Danielhiversen/pyTibber#288
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: