-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Convert Bosch BTH-RA to modernExtend
#7498
Conversation
@Koenkk What should we do regarding Interestingly enough, it does support |
modernExtend
modernExtend
What can we do here? I guess add the system modes which the device supports? |
I agree. This mean |
Great, if you can fix the failing CI I can merge this. |
I see your point too. Firstly, what speaks for your approach is that the device only publishes heat (and cool) states (0x04 and 0x03). So this is a formal approach. On the other hand, as you stated yourself, the manufacturer did not comply with the specification. Therefore, I do not see the benefit of trying to comply at all costs, especially if this does not improve the user experience but rather results in a setback: The implementation cannot map the state to the relevant and intended key but to some proprietary attribute.
You stated that this was better, but you did not say why. I just do not see the real benefit besides formal compliance.
I cannot find anything or anyone speaking against this understanding of "auto." So, I believe this point is valid and would still advocate for the implementation of pause and auto. It would align best with the specs and usability from a general perspective. Therefore, I kindly ask you to reconsider this decision and revert the change for the benefit of the normal user. I see that you provide the code and have made all the effort. So it seems natural that eventually, you decide on the way to go – the commit is merged anyway, and I am in no way technically capable of making an alternative PR.... So I tried to go on and implement the new situation in my setup. I made quite an effort (no IT professional here...). Unfortunately, I am facing some difficulties and am unsure how to proceed sensibly. Maybe I am misunderstanding it in the first place:
Yes, but as I understand it, the device will not work properly (out of the box) because it cannot read/set the operation_mode. So I have a rather big self-made heating control logic that depends on these states to determine if it should use automatic commands (auto/schedule) or manual override from the UI or even from the device itself, very handy (heat/manual). So this is not only (but also, @l3rdy) about the standard climate card not working anymore... So I do not see an option but to manually add it in MQTT. If this could be done in auto discovery, this would really be the BEST! and it would avoid the downsides and keep the new structure and benefits!? Thank you very much for the template you provided! I am not totally sure where this is meant to be implemented, though. I did set up an MQTT climate device, mostly derived from the autodiscovery message (without the template so far because I do not run the new converter):
This leads to a virtual device that shows mode (old version) and temperature, setpoint, etc. But it does not include the secondary entities. Is there an easy way to do this? Is the template meant to be used in this way? If there is no solution by reverting or auto-discovery, any guidance is much appreciated! |
We'll have to unfortunately draw a line somewhere.
Again, please see above. 👆
That's your interpretation of the spec, which is fine, but I disagree based upon my knowledge of similar specs and how HVAC modes are handled there.
I went this way because we have to adhere to something, ZCL in this case. We cannot start implementing converters just to cater to a single ecosystem and its idiosyncrasies. ZHC just isn't the right place for such specific stuff.
You can't (at least currently) alter the schedule on the TRV, you know this, right?
Now, what should we do? 🤔
Just add them to your
Well, seems to be a HA integration issue as well? I can't really help with these things here, sorry. |
I have just seen I didn't post my response I wrote a few hours ago, but I guess I might as well do it now. 😄
I think there's a big misunderstanding here. Everything should work out of the box of course. (Even in HA) Also gotta mention regardless that this Z2MQTT and not Home Assistant. So there might be situations where things (maybe?) could be better for Z2MQTT and not for HA in theory (tho I can't really think of one). There's People who use Z2M and not HA and the other way around. Should issues occur it should be within Z2M (where most definitely everything works) for HA issues we should switch to the HA community and look at it there, if there's one. As far as I can tell you and what I've seen, there's not really an Issue in HA. Everything gets exposed and works. Just slightly different. A mod to your cards and/or Automations and you should be good. So of course feel free to test the dev release and keep everyone updated if you run into issues within Z2MQTT. |
Happens to the best of us. 😅
Correct.
Preach! 🙏 I personally don't use HA because another "layer" adds to much latency for my taste. It's Z2M straight to HomeKit, with Node-RED tagged on for automations. HomeKit is more or less just the frontend and handles notifications, scenes, and home/away stuff.
Yes, support for Boost Mode, etc. is up to HA.
The key here is currently. 😉
✌️ |
As stated before, I can understand your point and can accept the decision. I just want to point out that my point of view is in no way related to home assistant, but only what should be done if the implementation of the manufacturer does not comply to standard. In my view, it then makes little sense to state that zcl is king, cause it was not in the first place. This is clearly true for pause/off, what is an operating mode and will be exposed as heat, whats just not accurate.
Please, dont be silly.
I did create the device as mqtt template entity and I was just wondering if there is another place just for the template in home assistant/auto discovery to specifically alter this property of the auto discovered device. Maybe I could write the whole config in mqtt.yaml (additional entities, registering to each device etc), but it will be clumsy, its really much copy and paste, cause scope of variables is too limited. Moreover it probably will just be unmanageable when future changes occur...
Looking forward to see this:
Thanks for your commitment in trying to improve the device support!
Best regards |
Thanks for understanding.
Interesting solution. I had some issues with the manual setpoint being randomly overridden by the schedule. The TRV firmware is still kinda messy.
Feel free to open a PR on the main Z2M repo, I unfortunately don't have the right setup to develop/verify such a patch. 🙈
Not an atomic clock (yet), but I run a GNSS-based time server. 😉 |
I tested the dev converter in my setup and it works nice. 😄 Just two minor things:
Implemenation in home assistant:
I do not feel like I was able to accomplish that cause I have to admit my knowledge of these things is just derived from trial and error and thus very limited. I do not understand the structure of the code in the auto-discovery and it would be way to risky to break everything. However, I checked the documentation and found out that i just can update the rendered values in auto-discovery with some mqtt commands. It seems to work for me, though I have not tested it thorouhgly. Its in basis just expanding and mapping of some keys, based on these values and templates:
And this is the corresponding example flow in node-red for one trv
Maybe this will be useful as a blueprint for others I am willing to do the update on the device documentation, but i am abroad for the coming 10d and this will have to wait. |
Yep, just a change in name.
Just wait a few hours. It takes some time for the first report to be sent.
That's how we learn. 😉
Looks good! Should work just as well as before.
Thanks for sharing!
Cool, take your time. 👌 |
Unfortunately, I still get the null value. |
Hmm, can you do me a quick favor and issue a read on |
Sorry for the delay, but i was not at home some days... I did log the readings on the cluster "powerConfig" → AlarmState. it is not the same on the devices: Device #2 Device #4 Device #5, Device #6, |
@dierochade I've removed We'll have to figure out which devices actually report |
@burmistrzak It would be of much more interest to be able to edit the builtin schedule. Have you already had the time and inclination to take a look at it? I did lookup the documentation to for the device to update it, but, that's already done... 😉 One other thing: I did the upgrade in my normal setup and noticed that I had made an error in the template provided ("operating_mode" in command topic and command payload), so it did not work. I did edit the original post and fixed the code. |
True, that's why it's gone now. 😉
I played around very briefly, but it I'll have to dig through my packet captures to get something that works.
Great, thanks! 🙏 |
Clarified new control of system_mode since [Convert Bosch BTH-RA to modernExtend #7498] (Koenkk/zigbee-herdsman-converters#7498)
Clarified new control of system_mode since [Convert Bosch BTH-RA to modernExtend #7498] (Koenkk/zigbee-herdsman-converters#7498) Reworked the commit so that it only affects the 'Notes' section. I hope everything is fine now. Thanks for your patience, I'm still trying to find my way around...
Clarified new control of system_mode since [Convert Bosch BTH-RA to modernExtend #7498] (Koenkk/zigbee-herdsman-converters#7498) Reworked the commit so that it only affects the 'Notes' section. I hope everything is fine now. Thanks for your patience, I'm still trying to find my way around...
Just to add my 2 cents, from a technical perspective this change makes perfect sense and needed to be merged. However as it completely breaks existing setups (Bosch BTH-RA is now simply not usable anymore in HA via the usual service calls), this merge and its release should really have been coordinated with an equivalent change to the auto detection template generation code in the HA integration. Just to say "this must be solved in the main Z2M repo and is therefor not our problem" is very shortsighted. This breaks a lot of trust, as really no normal user makes a distinction between the converters repo and the main repo. Z2M has a reputation of just working and this is the opposite of that. During my research yesterday into the reasons why my thermostats are not usable anymore I already stumbled upon a thread with the theme "do not use Z2M, use ZHA, as it just works. Noone needs the extra features of Z2M." For me personally, I can fix this locally by creating my own device configs inside mqtt for HA to detect, but in the future I will go through each change log of Z2M like a paranoid mad man to avoid issues like this. Downgrading to 1.37 might be another option and to never upgrade again. Shoutouts to @dierochade for creating the necessary templates, these will make my fixup work much easier! |
@gpayer I agree that a corresponding change to auto-discovery would have made for a much smoother transition. Btw. I even had a Z2M fork to implement this, however I wasn't really satisfied with my solution, and abandoned it shortly thereafter. Main reason being, that I didn't want to force a specific mode mapping onto HA users. Anyone who noticed the change and actually needs the legacy mapping, is free to use the provided template until we find a long-term solution. Also, you absolutely should read every changelog before updating any software! Lastly, please keep in mind that we're all volunteers, doing this on our own time, for no compensation other than enjoyment. Users of FOSS aren't entitled to anything more than what's stated in the license. |
@burmistrzak What stopped you from just providing the legacy mapping in the beginning and adding other modes later on? I also do not really understand the usecase for different mappings. Most often you just want your thermostat to work with your existing automations. And for a climate device to be considered a working climate device the service I mean, this is the whole point of the templating functionality in the MQTT integration. And if Z2M provides a auto discovery config for a MQTT HVAC device then this config should actually work. And in this case the provided config is simply wrong. That in of itself is a bug, there can be no discussion about it. Yes, you could set the operation_mode attribute manually, but then you would have to touch every existing automation regarding climate devices! And what if you have different models from different vendors? That's why you (and every existing third-party blueprint and custom integrations like versatile thermostat) use And yes, enjoyment is one currency in FOSS. However, especially in big and widely used projects, reputation and trust are currencies as well and I very much assume these are part of the enjoyment. tl;dr I understand your motivation to make the created mapping more flexible, but I do not understand your opinion that only a tiny minority actually uses climate devices in HA in the default way. And yes, I'm salty that I had to put in a few hours of work to get everything back on track again. Thankfully it's summer around here, in the winter I would have been royally screwed. I probably would have reverted back to Z2M 1.37 and never upgraded again. |
Well, it's a Bosch issue. The TRV simply doesn't support
We're just passing along what's actually supported by the device.
Guess, no currency for me then! 📉
Not reading release notes and then chirping at contributors is overall a solid way to get things done. 👏👏👏
Indeed, very tragic.
And..? |
First of all, I don't want to put down your work. Again, this change was necessary to be able to control this TRV precisely.
I know that, I was talking about the home assistant integration, the mqtt payload sent to a specific I've sent an adapted config manually into a new
Again, I was talking about the home assistant specific config sent to the I really don't get how the home assistant specific integration (homeassistant.js) is relevant to other ecosystems.
I already provided a constructive idea in the actual bug issue. You dismissed it because you want something more complicated. And I simply don't have the time atm to dig into yet another FOSS project. Furthermore I can't develop locally, I don't have a second zigbee usb stick. Is there a way to test with a mockup? Btw still waiting for the use case of having an "accurate mode" in HA! |
I do, but these things can (and will) happen in relatively fast paced projects.
Respectfully, no. You don't understand. There're a few things that make auto-discovery & Co. rather tricky: TL;DR
Sigh... See above.
Well, too bad. Guess you'll have to wait then, or find some time.
You can write e.g. a unit test, and develop against it.
... |
@burmistrzak, first of all, thanks for taking care ❤️ I think that there is a lot of work in the background we cant even imagine... There are two hearts beating in me. As I am a professional doc writer, I read release notes but I have to admit I did not got the impact of the changes. Thanks to @dierochade for preparing some readme updates to BOSCH-BTH. The culprit of all that is as far I understand that Bosch does not stick to specs which is a shame to a central European, in particular German company. I think that a lot of things could have been relaxed if the old vs. new situation would have been described in a more user consumable way than in a dev view. On the other hand side, as a user, I got "zillions" of HA log entries telling Reading the posts, I did the following to silence the HA logs: In Zigbee2MQTT (runs as container outside HA), I switched settings back and forth in Operating mode fields respectively clicked on the System mode field on both devices. Now, I have no warning log entries anymore. |
@mmattel Thank you for your kind words. 🫰 You are correct, Bosch is indeed not following spec. I personally suspect that this is deliberate to limit interoperability, because 99% of features can easily be implemented with standard ZCL, no custom clusters necessary. In fact, it's more effort to do custom stuff! 🥸 Btw. I already have a rough proof-of-concept for a possible solution that, while certainly not perfect, should automatically restore the previous behavior for HA users. |
Reimplementation of the Bosch TRV using
modernExtend
, and includes the following additional changes:operating_mode
genPollCtrl
clusterbattery_low
off
&auto
fromsystem_mode
Even though the TRV supports a cooling mode (yup..), it's currently not implemented here.
ℹ️ Fresh off the press, may require hotfixes.