-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
Enhance support for Vasco and ClimaRad HRUs #133
base: master
Are you sure you want to change the base?
Conversation
Thanks for your contribution, I am looking forward to merging it in. You'll see that the PR did not pass CI, are you running the pre-commit, mypy and pytest?
... all these should pass before submitting a PR. Second, the PR should be as small as possible, and atomic. If something could be split off as a separate PR, generally it should be: the spelling typos are once such example. |
OMG, just adding codespell - is full of typos - I will add codespell, fix many of these in my own PR. (you'll have to rebase) |
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 is a first step. We can look again, once I have time to understand the detail.
ITHO__ = "itho" | ||
NUAIRE = "nuaire" | ||
ORCON_ = "orcon" | ||
VASCO_ = "vasco" | ||
CLIMARAD = "climarad" | ||
|
||
TEST_SUITE_300 = [ | ||
# { # THM to CTL: FIXME: affirm is I|1FC9|07, not I|1FC9|00 |
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.
You need to add some test bindings of these systems. Look at ./tests/bindings/hvac/*
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.
Bindings are further down the To Do list. Have to climb on top of our bathroom to push a button on the MiniBox...
Took these lines out of the 22F1 PR #136 .
…vasco # Conflicts: # src/ramses_rf/binding_fsm.py
Thanks for the educational pointers.
On my PR branch I get 8 more, but still. As a novice in |
I can reproduce this... the solution is to
It is not immediately clear to me why the existing pyproject.toml isn't sorting it out without the |
OK, I have had a deeper look now, and here's a way forward... It appears you have updated the parsers for at least the following command codes:
I am going to ask you to submit multiple PRs - one for each code. You will also need to add packets to the corresponding tests. For example, look at:
In addition, it will be very helpful if you have a log of your remotes/sensors binding to your fans. This will help with faking. And also a
|
For my own reference, learned this on Stackoverflow:
Sorting out my Python setup now. |
Perfect approach, thanks. TBC |
You can leave this PR here for now - the TODO list is useful. As I said in #134 - start with the codes, one PR per code - a number of smaller PRs is much easier to get merged (for both of us). You don't have to do one at a time, you can have several codes on the go at once, but start with a simple one, and you know what to expect to get your PR merged (e.g. including your new payloads in the test suite). |
All open PR's in the above list are now ready for your review |
Current
|
@zxdavb Any chance we can see these changes in a ramses_cc update before year's end? |
I am sorry, but it looks unlikely I'll have that much time. What is the feature you're wanting most? |
That would be |
Proposed additions to process messages from a Vasco D60 HRU (2022), a ClimaRad VenturaV1x HRU (2021) and a ClimaRad MiniBox fan that I have installed in my home.
I have ramses_rf communicating via ramses_esp on an ESP32S3 board with a C1101..
Not all commands as entered in the ramses-rf CLI cause a change on the units, but at least reporting from the units works. The message contents are not shared by the manufacturer and especially for the Ventura they customized them. The PCBs are all Airios OEM.
Many messages from the units and their remotes are rejected by ramses-rf, so I cannot run Home Assistant with ramses-cc at the moment.
Hope to have made it a bit easier to add suport for these models in ramses-rf. The typed stuff is a bit too much so I cheated somewhat there. Fixed some typos in comments across your module while browsing.
Added a packet log below. I can provide more info upon request.
Cheers, Egbert
packet3.log