Added typing hints and type checking across the board + implementation of "RepeatClimateReactAction" feature

[seidnerj]

I was working on a small feature but got carried away and eventually typed and introduced type checking for almost the entire project (but unfortunately did not do any work on the original feature I had planned, oh well...). This took a while... BUT - I believe everything is working properly. I can't be sure though since I couldn't test everything given I do not have all the types of devices the plugin supports.

The great thing about this change, is that when one uses VSCode, pretty much everything is typed which really really helps avoiding mistakes, and hence prevents introducing bugs. I guess the next step is just move everything to TypeScript but we'll leave this for the future.

@benwebbbenwebb @nitaybz I realize validating this PR is going to be a lot of work, but I hope it'll get merged since it should make working on, developing and improving this plugin much much (!) easier.

I'm currently in the testing phase, but thought it made sense to already share this while I complete the testing to the best of my ability.

Thanks in advance!

[benwebbbenwebb]

Hi @seidnerj

Let me know when you have (mostly) finished - I’ve seen you make some additional updates every week or so - and I’ll start to review the changes.

[seidnerj]

Hey @benwebbbenwebb, I've been running this version for a while now. Everything (that I can test) seems to work as expected. I don't own all types of devices so can't test everything. Apart from that, looks like it's ready for prime time 🙂

[benwebbbenwebb]

First review done @seidnerj, please take a look at the comments.

Sorry it took so long!

I've also created a release branch from the current v2.5.0 (in case of any pressing issues with the current release) so once you've reviewed the feedback we should be able to merge this PR to master pretty soon (to prevent further merge conflicts etc) and then do an alpha release.

[benwebbbenwebb]

Hi @seidnerj

Added some additional comments / suggestions

There also look to be a few pre-existing comments still open for discussion

[benwebbbenwebb]

What's the point of climateReactAutoSetupOffset if its default is 0 (zero)? How do you expect people to use it and climateReactAutoSetupMultiplier together?

To simplify it I'm wondering if the following is better:

Suggested change

	smartModeState.highTemperatureThreshold = device.state.targetTemperature + (device.usesFahrenheit ? 1.8 : 1)*climateReactAutoSetupMultiplier + climateReactAutoSetupOffset
	smartModeState.highTemperatureThreshold = device.state.targetTemperature + climateReactAutoSetupOffset

Give user complete control over how many degrees above (or below), and have a default of 1 on climateReactAutoSetupOffset. Note: the descriptions in README and config schema are (currently) written such that Offset is the main one, Multiplier is placeholder.

We may also need to round the resulting Fahrenheit value as I'm not confident that Sensibo supports decimals on the temp thresholds when in F (like it does in C).

[benwebbbenwebb]

Should this be:

Suggested change

	smartModeState.lowTemperatureThreshold = device.state.targetTemperature - (device.usesFahrenheit ? 1.8 : 1)*climateReactAutoSetupMultiplier + climateReactAutoSetupOffset
	smartModeState.lowTemperatureThreshold = device.state.targetTemperature - ((device.usesFahrenheit ? 1.8 : 1) * climateReactAutoSetupMultiplier + climateReactAutoSetupOffset)

Otherwise wouldn't the Offset increase rather than decrease the threshold?

[seidnerj]

Nope, this is fine. I'll explain the rational - the point of the offset is to allow something like this:
For example, when setting cool 25C, instead of keeping the temperature between 24 and 26, with an offset of 1, you can have it kept between 25 and 27. I actually use that for that exact purpose because I rather the AC stop cooling immediately when it reaches below 25 but also don't want it to start it at 26 because then it starts and stops every 5 minutes which drives me crazy. I rather have it start at 27 because then it only starts and stops every 10-15 minutes or so.

[benwebbbenwebb]

I think I now understand what you are aiming for, but it probably needs to be reworded to ensure others understand it (especially as I didn't the first time).

I'm probably struggling the most with 'multiplier'... maybe the settings should be called range and offset instead?
Offset would be defined as degrees above (or below) current AC temp to start (or stop), allowing your use case of 0 offset (and mine of -1)
Range would be defined as degrees +/- from that starting point to set the other end of the threshold.

I'm imagining:
Mode Cool, temp 25
Offset = 0
Range = 2
lowTemperatureThreshold = 25 + 0 (offset) (range not used) = 25
highTemperatureThreshold = 25 + 0 (offset) + 2 (range) = 27
AC turns on (set to 25) at 25 and turns off at 27

Mode Cool, temp 24
Offset = -1
Range = 2
lowTemperatureThreshold = 24 + -1 (range not used) = 23
highTemperatureThreshold = 24 + -1 + 2 = 25
AC turns on (set to 24) at 23 and turns off at 25

Mode Heat, temp 20
Offset = 0
Range = -3 (for Heat mode we could "simplify" it and have the negative implied, saves users needing to change their settings between seasons)
lowTemperatureThreshold = 20 + 0 + -3 = 17
highTemperatureThreshold = 20 + 0 (range not used) = 20
AC turns on (set to 20) at 17 and turns off at 20

Mode Heat, temp 22
Offset = 1
Range = -2
lowTemperatureThreshold = 22 + 1 + -2 = 21
highTemperatureThreshold = 22 + 1 (range not used) = 23
AC turns on (set to 22) at 21 and turns off at 23

[benwebbbenwebb]

Also still need to check how Sensibo API handles F degrees in "SmartMode"... i.e. do we need to change F to C

[seidnerj]

Sure - will take a look when I have a bit of time.

[seidnerj]

I think I addressed everything either directly or indrectly, lmk if you have any questions.

[benwebbbenwebb]

Hi @seidnerj

Sorry it took me so log.

There is still a number of unresolved comments and changes requested (including at least a few that I believe would introduce bugs if not made), especially in the bigger files, e.g.:

• AirConditioner
• StateHandler
• StateManager
• api
• refreshState
• unified

[benwebbbenwebb]

Also still need to check how Sensibo API handles F degrees in "SmartMode"... i.e. do we need to change F to C

[benwebbbenwebb]

Suggested change

	return this.getFanSpeedForFanLevel(device, acState.mode, acState.fanLevel)
	return this.getFanSpeedForFanLevel(device.remoteCapabilities.modes[acState.mode], acState.mode, acState.fanLevel)

OR

Suggested change

	return this.getFanSpeedForFanLevel(device, acState.mode, acState.fanLevel)
	return this.getFanSpeedForFanLevel(device)

[seidnerj]

Hey @benwebbbenwebb, I'm not sure when I'll get to this, I'm swamped at the moment, will work on this when I can. Sorry!

[benwebbbenwebb]

No worries at all @seidnerj, I totally understand as I’m often the same!

And sorry for the pushback, I’m being quite cautious as I don’t want to introduce any issues that may then take more time to hunt down (when we are busy with other things).

[seidnerj]

No worries at all, I just didn't expect this up this much time - I am currently using this version without any real issues on my end, but then again - I don't use every single feature. I completely understand your stance, I just don't have the time at the moment to work on it. Hopefully in a few weeks. Will update when I can.