home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
71.17k stars 29.85k forks source link

Modbus - climate entity scale affects multiple registers #105932

Closed gfrancesco closed 9 months ago

gfrancesco commented 9 months ago

The problem

I'm using the climate entity to control the hot water temperature of my heat pump. It works fine, there's just a minor problem with how the current hot water temperature of the tank is displayed. Here's a picture of the card: Screenshot 2023-12-17 at 22 54 50

I write the setpoint (48 ºC) on a register, while I read the tank temperature from another register.

The problem is that the setpoint register is using whole numbers, while the tank temperature is using a register that reads 4860 in this case, when the temperature is 48.6.

I tried adding the scale: 0.01 configuration variable, however it scales both the tank temperature (that is now correct) and the setpoint variable (that is now wrong). Is there a way to apply the scale factor only to the tank temperature?

Unrelated, but there are also some mistakes in the docs: the descriptions of the configuration variables of the switch entities refer to the fan. Looks like a copy/paste mistake, but can be confusing for the reader.

Thanks!

What version of Home Assistant Core has the issue?

core-2023.12.3

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

Modbus

Link to integration documentation on our website

https://www.home-assistant.io/integrations/modbus

Diagnostics information

No response

Example YAML snippet

- name: PDC-1_DHW
      slave: 1
      address: 43
      input_type: input
#     scale: 0.01
      target_temp_register: 10
      temp_step: 1
      min_temp: 30
      max_temp: 55
      hvac_mode_register:
        address: 12
        values:
          state_off: 0
          state_heat: 1

Anything in the logs that might be useful for us?

No response

Additional information

No response

home-assistant[bot] commented 9 months ago

Hey there @janiversen, mind taking a look at this issue as it has been labeled with an integration (modbus) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `modbus` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign modbus` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


modbus documentation modbus source (message by IssueLinks)

janiversen commented 9 months ago

Pull requests are welcome, especially if the documentation is wrong.

One register can only have one type, that is pr modbus protocol definition, so it strange if your device have different types depending on read and write.

gfrancesco commented 9 months ago

It is not one register, they are two different ones, see the yaml. 43 is the sensor and 10 is the setpoint.

janiversen commented 9 months ago

those are both temperature registers and are thus treated identically.

As far as I can see the integration works as documented, and therefore it is not a bug, but a feature request. Issues are for bug, feature requests is something you need to gather momentum for in the user forum, or even better submit a PR.

Your problem is easily solved with a climate template, no need to complicate the configuration in my opinion.

Closing due to not being a bug.

gfrancesco commented 9 months ago

Well, if the fact that temperature registers should all be treated identically is a design choice, than ok it's not a bug. However that breaks pretty much all Daikin heat pumps using a modbus gateway (and other modbus devices) since the fact that two registers measure temperature does not mean they represent the number in the same way, see the example in my first post.

As per the documentation, there is no indication to which register the scale, offset and precision are applied to. Thus it is also difficult to distinguish the expected behaviour from bugs. In fact there are some variables applying to the target_temp register that use the target_temp prefix (e.g. target_temp_write_registers), so I expect any other variable affecting that register to also use the same prefix, but that's not the case.

I'll try the climate template, but if this is the integration your are referring I want to point out that is a custom integration unsupported by HA.

janiversen commented 9 months ago

I am referring to templates that are part of HA, I would not recommend a custom component here (even though I highly respect custom components developers).

You might be right it would make sense to make such a change, and pull requests are welcome

gfrancesco commented 9 months ago

Ok, then it cannot be easily solved with a climate template, or any template that is part of HA. People that had my exact problem have tried in the last months and is not fixable.

janiversen commented 9 months ago

You have other possibilities like actions. But independent if you want a change feel free to make a pull request, that is how open source works.