Skip to content
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

BRT-100-TRV improvements #4832

Merged
merged 4 commits into from Oct 30, 2022
Merged

Conversation

xworld21
Copy link
Contributor

I am trying to improve the BRT-100-TRV converter a bit. In short, I did:

  • complete the climate interface, in so far as possible: expose system_mode (always heat) and running_state (duplicate of valve_state). This is mostly to the benefit of Home Assistant: with this PR, HA reports the heating/off state directly in the thermostat widget, and correctly shows "heat" as the only mode (as opposed to heat/cool/auto/etc)
  • fix a couple of inaccurate temperature limits
  • change the temperature step to 1, as the protocol only allows integer values

I hope the changes are ok. I don't particularly like the way I handled system_mode. The valve does not report anything useful for it, as its value would always be "heat" anyway, so I tacked the system mode value to the preset. Is there a better way to expose a constant value?

@xworld21
Copy link
Contributor Author

My bad, I should have run the linter! Will do next time.

How do I get around the no-constant-condition check?

converters/fromZigbee.js Outdated Show resolved Hide resolved
converters/toZigbee.js Outdated Show resolved Hide resolved
While the TRV supports half degree temperature setpoints, the Zigbee
protocol only supports integer values, both for setting and reporting.
@Koenkk
Copy link
Owner

Koenkk commented Oct 30, 2022

Thanks!

@callacomp
Copy link

callacomp commented Nov 2, 2022

This has broken my TRVs. They are say unknown now. In the operation dropdown I can now only see heat but if I select it it changes back to unknown again, error in mqtt
`Logger: homeassistant.components.mqtt.climate
Source: components/mqtt/climate.py:573
Integration: MQTT (documentation, issues)
First occurred: 10:24:49 (18 occurrences)
Last logged: 10:40:44

Invalid modes mode:`
There's also no info in Z2M screen for new fields

system_mode
Mode of this device
N/A
running_state
The current running state
N/A

Look into it later, got to work!

@Impact123
Copy link
Contributor

change the temperature step to 1, as the protocol only allows integer values

Why? The thermostats supports 0.5 increments and this seems to have been working perfectly fine.

@xworld21
Copy link
Contributor Author

xworld21 commented Nov 3, 2022

change the temperature step to 1, as the protocol only allows integer values

Why? The thermostats supports 0.5 increments and this seems to have been working perfectly fine.

Fractional values are not exposed over Zigbee:

key: ['current_heating_setpoint'],
convertSet: async (entity, key, value, meta) => {
const temp = Math.round(value);
await tuya.sendDataPointValue(entity, tuya.dataPoints.moesSheatingSetpoint, temp);

case tuya.dataPoints.moesSheatingSetpoint:
return {current_heating_setpoint: value};

so from the Z2M point of view, only 1 increments are supported in practice. That's why if you try to set e.g. 20.5, it will snap to 21. The converter is rounding up before sending the value. So we might as well declare than only integer increments are supported.

@Impact123
Copy link
Contributor

Thanks for that. I just checked again and you're right. I could've sworn this was working some time ago though

@Heire
Copy link

Heire commented Nov 3, 2022

This update indeed broke my TRV also:
system_mode
Mode of this device
N/A

And I do agree with @Impact123 that I could put 0.5 degrees in Z2M before cause I have a scheduler still configured to push it to 20.5° instead of rounded numbers. Still remains the question how come this worked in the past if Zigbee doesn't allow fractional numbers. 0.5 seems nothing but it's actually a difference in heating the room just too warm cause of the delay in heating up the room ;)
Anyway the system_mode issue is blocking now the TRV.

@xworld21
Copy link
Contributor Author

xworld21 commented Nov 3, 2022

For everybody affected by this PR: there is a workaround at Koenkk/zigbee2mqtt#14771 (comment) (which is, flip between preset & manual once so that the TRV reports a value for system_mode, once the value is there, it should never disappear again). The issue has been reported at Koenkk/zigbee2mqtt#14771 and I'll work on it this weekend.

Re 0.5, I suggest opening an issue? I'll git blame my way through, if it worked in the past, the code must be somewhere in the history. I would also like to set fractional temperatures if that's possible!

@Heire
Copy link

Heire commented Nov 3, 2022

Yup perfect, I tried changing values to get an update but never thought to click on that one.
Works again like normal :)

Regarding the 0.5 have a look if you can, I know there have been other discussions on the 0.5 but each time that it only allows on 1° difference. However I'm pretty sure this TRV allowed 0.5° to be put in Z2M.

@Heire
Copy link

Heire commented Nov 4, 2022

Just a follow up on the 0.5 degree part, I have another TRV from another brand which allows a 0.5
Received MQTT message on 'zigbee2mqtt/Verwarming Bureau/set' with data '{"current_heating_setpoint":18.5}'
Publishing 'set' 'current_heating_setpoint' to 'Verwarming Bureau'
Received Zigbee message from 'Verwarming Bureau', type 'commandDataResponse', cluster 'manuSpecificTuya', data '{"dpValues":[{"data":{"data":[0,0,0,185],"type":"Buffer"},"datatype":2,"dp":2}],"seq":53533}' from endpoint 1 with groupID 0
Info 2022-11-04 09:53:29
When I look at the Moes one:
Publishing 'set' 'current_heating_setpoint' to 'Verwarming Keuken'
Received Zigbee message from 'Verwarming Keuken', type 'commandDataResponse', cluster 'manuSpecificTuya', data '{"dpValues":[{"data":{"data":[0,0,0,20],"type":"Buffer"},"datatype":2,"dp":2}],"seq":22}' from endpoint 1 with groupID 0
So indeed there is no fractional decimal possible but it uses a 3 digit to allow the decimal to be used while in the new release you push a 2 digit. I guess the TRV will see the difference.
Something worth looking at, no?

@xworld21
Copy link
Contributor Author

xworld21 commented Nov 5, 2022

I have done a minimum of digging... Support for the BRT-100-TRV was merged Jun 2021 in #2652. As far as I can tell, the code was never able to set half degrees. The explicit rounding up was introduced after two weeks, I am pretty sure because fractional values were triggering a bug in a TuYa function that expected integers (commit & PR don't offer details, so I can only conjecture). Given my (little) experience with TuYa-based devices, there is no way to set a non-integer setpoint. The firmware doesn't expose the functionality over Zigbee to do that.

On the bright side, I think the schedule does support half degrees, because those values are multiplied by two, except the current converter only does integers, apparently on purpose. I'll try to update the schedule code, if it works, you'll see a PR.

@xworld21 xworld21 deleted the brt-100-trv-emulate-zcl-hvac branch November 7, 2022 11:55
Mephistofeles pushed a commit to Mephistofeles/zigbee-herdsman-converters that referenced this pull request Dec 13, 2022
…k#4832)

* BRT-100-TRV: add constant system_mode

* BRT-100-TRV: duplicate valve_state to running_state

* BRT-100-TRV: improve accuracy of temperature limits

* BRT-100-TRV: change temperature step to 1

While the TRV supports half degree temperature setpoints, the Zigbee
protocol only supports integer values, both for setting and reporting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants