Mosibi / Midea-heat-pump-ESPHome

Apache License 2.0
53 stars 14 forks source link

Update Register 2, 4, 6 values min_max with checking Register 2 min/max which depends on emmission type installed #60

Closed rysiulg closed 2 weeks ago

rysiulg commented 3 weeks ago

hinot exactly:)the value range depends on emission type is huge:)eg user have emission type radiant so he isn’t allowed to put value below 18 but without this user can send value 5 to heat pump -so in return on next read he theoretically get this minimal value of 18 -even he send 5.so this part adjust and limits to send incorrect -not accepted value to heat pump. user feedback is similar with this part of code like without. these hard coded values are max and min values accepted by heat pump. there isn’t possibility in this moment with esp home to send this info for user -ideally would be to hardcode this to get min and max value -but for this moment esp home don’t support lambda calls in min and max and only solution is build for these sensors own full part of code in loop section…this in future could be more help to add lambdas for min and max and then it could be removed from lambda write part;)but tyou are owner of basic code -so if you want it could be removed -no problem, but also very wide range of temperatures persist 5-60personally i use 5 deg in summer and in winter time 27-44deg ;)PozdrawiamRyszard GwizdakMARM.pl Sp. z o.o.Wiadomość napisana przez Richard Arends @.***> w dniu 26.10.2024, o godz. 17:43: @Mosibi commented on this pull request.

In heatpump.yaml:

@@ -3189,23 +3078,49 @@ number: value_bytes[0] = value >> 8; // high byte (zone 2) value_bytes[1] = value & 0x00FF; // low byte (zone 1)

Rereading your code helped 😉 What you do is set the limit based on the emission type. I see two issues with this:

The user sets a value, but the system changes that value without feedback to user. If the user does not verify, another value can be used then he/she wanted. The limits are hard coded and are easily forgotten if these are changed somewhere else.

I rather keep it simple and only define the upper and lower limit, thus as it already is.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

Mosibi commented 3 weeks ago

I will keep this in mind, but for now let's move it out and revert it.

Maybe in the (near) future we can introduce a global var that defines the limits for the specific emission types. Then we can use those vars in both the default range and in the code. That way we only have to change it at one spot.

rysiulg commented 3 weeks ago

oki will revert nowno you can’t neither set min_value and max_value values. it should be done by make pull request for new feature to esp home -you can’t update these values by lambdaPozdrawiamRyszard GwizdakMARM.pl Sp. z o.o.Wiadomość napisana przez Richard Arends @.***> w dniu 26.10.2024, o godz. 19:44: I will keep this in mind, but for now let's move it out and revert it. Maybe in the (near) future we can introduce a global var that defines the limits for the specific emission types. Then we can use those vars in both the default range and in the code. That way we only have to change it at one spot.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

Mosibi commented 3 weeks ago

@rysiulg I am a bit busy lately, this weekend I will look into your PR

Mosibi commented 2 weeks ago

@rysiulg merged. Again thank you for your contribution !