-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
idea with weekdays programs for tuya-moes thermostats #8009
base: master
Are you sure you want to change the base?
idea with weekdays programs for tuya-moes thermostats #8009
Conversation
I'm not using HA and any other visual dashboards except z2m so for me this whistles is important :) |
return Math.max(min, Math.min(max, value)); | ||
}; | ||
|
||
return [clamp(parseInt(hourMinute[0]), 0, 23), clamp(parseInt(hourMinute[1]), 0, 59), clamp(parseInt(hourTemperature[1]) * 2, 5, 35)]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here is not needed exception. Clamp is more than enough.
@@ -122,7 +118,7 @@ const definitions: DefinitionWithExtend[] = [ | |||
legacy.tz.moes_thermostat_deadzone_temperature, | |||
legacy.tz.moes_thermostat_max_temperature_limit, | |||
legacy.tz.moes_thermostat_min_temperature_limit, | |||
legacy.tz.moes_thermostat_program_schedule, | |||
legacy.tz.moes_thermostat_program_schedule_v2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not call it v2 but update the existing one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is just draft but if you like the idea I'll change both moes/moesS thermostats for using this one with proper namings
(made pr as draft)
}; | ||
|
||
const getNumbers = (input: exposes.Text): number[] => { | ||
input.checkPatternMatch(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this does, how can input
be exposes.Text
here? Isn't it a string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aww
Sure. I've just started to dive into it and missed some core concepts. I've thought there is component and not clean result.
But maybe you'll suggest where can I make this check? I wanted to try to incapsulate all info inside component but not sure where this information can be used. As I see a lot of this stuff doing in z2m-frontend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would you do a simple regex check here instead of input.checkPatternMatch();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can but main idea was to improve something and provide kind of better incapsulated solution
Just an idea but maybe we can go with it and improve.
Would be great to make proper control for this kind of scheduling because whole set of available thermostats have it.
Now we have 2 implementations and bot not much useful:
– long string for all values
– bunch of separate fields