Update Characteristic value when setProps called

[Shaquu]

Update Characteristic value when setProps called

:recycle: Current situation

This PR is mainly about number values. Currently, Characteristic default value is based upon minValue. Once setProps is called and minValue is changed, then also value should be changed accordingly.

:bulb: Proposed solution

public setProps(props: Partial<CharacteristicProps>): Characteristic {
  //...

  if (this.props.format) {
    switch (this.props.format) {
      case Formats.BOOL:
      case Formats.STRING:
      case Formats.DATA:
      case Formats.TLV8:
      case Formats.DICTIONARY:
      case Formats.ARRAY:
        break;
      default:
        if (this.value) {
          const minValue = this.props.minValue ?? 0
          const maxValue = this.props.maxValue ?? minValue
  
          if (this.value < minValue) {
            this.updateValue(minValue)
          } else if (this.value > maxValue) {
            this.updateValue(maxValue)
          }
        }
    }
  }	 

  return this;
}

Problem that is solved

This PR will clamp Characteristic value, so there is no situation when allowed value range is 10-20 and value is 0.

Implications

updateValue is called, which means some actions might be triggered

Testing

Tested locally before and after with success.

Reviewer Nudging

Right at the top :)

[Shaquu]

@Supereg I will have a look at this soon, thanks for comments!

[Shaquu]

Can you have a look @Supereg? Also looks like there are some issues with tests on beta branch.

[coveralls]

Pull Request Test Coverage Report for Build 3247995066

• 9 of 10 (90.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.06%) to 51.788%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/lib/Characteristic.ts	9	10	90.0%
<!--	Total:	9	10

Totals
Change from base Build 3095726760:	0.06%
Covered Lines:	5724
Relevant Lines:	10098

---

💛 - Coveralls

[Shaquu]

@Supereg can I ask for hints or comments?

[Shaquu]

@Supereg bump

[ebaauw]

Did you guys check this change for Programmable Switch Event and other eventOnly characteristics, if there are any? I fear this might cause a ghost button action.

I now get the “characteristic was supplied an illegal value” on restart, when having changed the supported button actions, and the last button action before the restart is no longer supported. I thought about changing the characteristic value when changing the properties, but don’t know how to do that without generating a ghost button action.

[bauer-andreas]

My intention was to add some test cases in a follow up PR. Thanks for the input 👍

I now get the “characteristic was supplied an illegal value” on restart, when having changed the supported button actions, and the last button action before the restart is no longer supported.

You are justing calling setProps on a ProgrammabelSwitchEvent characteristic? Or am I misunderstanding something?

[ebaauw]

Yes. Based on the config.json settings, a button might support Double Press and/or Long Press in addition to Single Press. E.g. the doublePressTimeout and longPressTimeout settings in Homebridge RPi, or the hueDimmerRepeat setting in Homebridge Hue.

On restart, Homebridge restores the Stateless Programmable Switch from cache, including the last button action as the value of Programmable Switch Event. My plugin sets the characteristic properties based on the changed config.json settings. When the last button action from before the change is now no longer supported, Homebridge complains that the (restored) value is out of bound.

I think for eventOnly characteristics, there should be no check on setProps(), only on setValue(), updateValue(), and equivalents. HomeKit won't ever read the characteristic, so it won't see the out of bound value anyways.