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

"Target Heating Cooling State" Console Warning / bugfix #1

Open
TJ178 opened this issue Sep 19, 2023 · 4 comments
Open

"Target Heating Cooling State" Console Warning / bugfix #1

TJ178 opened this issue Sep 19, 2023 · 4 comments

Comments

@TJ178
Copy link

TJ178 commented Sep 19, 2023

Bear with me here, I haven't really contributed to open source stuff before so I'm not sure what the best format is :)

Added this plugin for the first time today, immediately worked except for this warning message:

[18/09/2023, 18:06:32] [ScreenLogic] updateTargetHeatingCoolingState: 2 {
  displayName: 'Pool Heater',
  bodyType: 0,
  minSetPoint: 4.444444444444445,
  maxSetPoint: 40
}
[18/09/2023, 18:06:32] [homebridge-pentair-screenlogic] This plugin generated a warning from the characteristic 'Target Heating Cooling State': characteristic was supplied illegal value: number 2 exceeded maximum of 1. See https://homebridge.io/w/JtMGR for more info.
[18/09/2023, 18:06:32] [homebridge-pentair-screenlogic] Error: 
    at TargetHeatingCoolingState.Characteristic.characteristicWarning (/var/lib/homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:3011:105)
    at TargetHeatingCoolingState.Characteristic.validateUserInput (/var/lib/homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:2927:14)
    at TargetHeatingCoolingState.Characteristic.updateValue (/var/lib/homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:2328:20)
    at Thermostat.Service.updateCharacteristic (/var/lib/homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Service.ts:795:35)
    at ThermostatAccessory.updateTargetHeatingCoolingState (/var/lib/homebridge/node_modules/homebridge-pentair-screenlogic/src/thermostatAccessory.ts:149:18)
    at ScreenLogicPlatform.updateAccessories (/var/lib/homebridge/node_modules/homebridge-pentair-screenlogic/src/platform.ts:352:38)
    at ScreenLogicPlatform.refreshStatus (/var/lib/homebridge/node_modules/homebridge-pentair-screenlogic/src/platform.ts:314:12)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

Dug into this a little bit and looks like there's a mismatch between the max value that we're feeding to homebridge and the actual TargetHeatingCoolingState outputs:

From src/thermostatAccessory.ts:69:

// register handlers for the TargetHeatingCoolingState Characteristic
    this.service
      .getCharacteristic(this.platform.Characteristic.TargetHeatingCoolingState)
        .setProps({
          maxValue: this.platform.Characteristic.TargetHeatingCoolingState.HEAT,
        })
      .on('set', this.setTargetHeatingCoolingState.bind(this))

Looks like we send the max value as TargetHeatingCoolingState.HEAT, defined as 1, wheras the actual max value that is used is TargetHeatingCoolingState.AUTO, which is defined as 3.

From src/platform.ts:533:

  /** map pool heat mode to thermostat target heating/coooling state  */
  mapHeatModeToTargetHeatingCoolingState(poolHeatMode: number) {
    switch (poolHeatMode) {
      case Controller.HEAT_MODE_OFF:
        return this.Characteristic.TargetHeatingCoolingState.OFF
      case Controller.HEAT_MODE_HEAT_PUMP:
        return this.Characteristic.TargetHeatingCoolingState.HEAT
      case Controller.HEAT_MODE_SOLAR_PREFERRED:
        return this.Characteristic.TargetHeatingCoolingState.AUTO
      case Controller.HEAT_MODE_SOLAR:
        return this.Characteristic.TargetHeatingCoolingState.COOL
      default:
        return this.Characteristic.TargetHeatingCoolingState.OFF
    }
  }

With this in mind, either we need to change the max value to be TargetHeatingCoolingState to AUTO or change the way it handles the mapping - it's a little strange how "cooling" is actually solar, but I suppose that's the best that can be done given HomeKit's limitations.

I can make a PR with the necessary changes, just wanted to open up discussion on the best way to do it.

Thanks :)

@zyonse
Copy link
Owner

zyonse commented Sep 23, 2023

My system doesn't have solar. Would setting it to AUTO still work properly for configurations like mine?

@TJ178
Copy link
Author

TJ178 commented Sep 28, 2023

Hmm, I doubt your system would respond properly without solar. Given that the labels are incorrect anyways, it might not be a bad idea to disable the AUTO and COOL options functionally, and create a parameter that allows for HEAT to be defined as one of the 3 different heating modes.

At the bare minimum, it would be fine to read all of the controller heat modes as HEAT in the HomeKit side.

@adriggs81
Copy link

Just recently started getting this error. Do you have steps to fix? Not family with modifying what you have above and implementing. Thx!!

@astrostl
Copy link

astrostl commented Jul 6, 2024

[homebridge-pentair-screenlogic] This plugin generated a warning from the characteristic 'Target Heating Cooling State': characteristic was supplied illegal value: number 2 exceeded maximum of 1. See https://homebridge.io/w/JtMGR for more info.

I'm just getting started with Homebridge and also have this warning spraying into my status/logs. I'm new to my own pool system as well, but I believe it has both gas and heat pump heating.

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

No branches or pull requests

4 participants