Update Register 2, 4, 6 values min_max with checking Register 2 min/max which depends on emmission type installed #60
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Register 2 with checks for min and max allowed temp depends for emmission types -unfortunatelly it is impossible to set min_value and max_value in lambda ;(
…ensor I'm sorry -i was missed some Register 210 bits that is ReadOnly -this move these back to binary sensors ;)
Changed in lambda mapping array position to if condition check to avoid error in array select ;)
Mosibi
reviewed
Oct 26, 2024
heatpump.yaml
Outdated
| @@ -3189,23 +3078,49 @@ number: | |||
| value_bytes[0] = value >> 8; // high byte (zone 2) | |||
| value_bytes[1] = value & 0x00FF; // low byte (zone 1) | |||
|
|
|||
| // ESP_LOGI("","Zone 1 is %d", value_bytes[1]); | |||
| // ESP_LOGI("","Zone 2 is %d", value_bytes[0]); | |||
| auto operation_state = id(${devicename}_operational_mode).state; | |||
There was a problem hiding this comment.
Can you explain what this code exactly does. I seems to me that the value (read and write) is changed based on the emission type.
We should not interfere with the input of the user, if the user specifies a certain value, then that value should be used. We must only limit the value of the slider, based on the limits of the specification in the manual
There was a problem hiding this comment.
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.
Commented in lambda ESP_LOG
restoring extracting 4-bit values
Mosibi
reviewed
Oct 26, 2024
corrections of zones decode
Mosibi
reviewed
Oct 26, 2024
remove reg:272 bit 12-15 mapping global from x ;)
removed extra line
|
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)
- // ESP_LOGI("","Zone 1 is %d", value_bytes[1]);
- // ESP_LOGI("","Zone 2 is %d", value_bytes[0]);
+ auto operation_state = id(${devicename}_operational_mode).state;
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: ***@***.***>
|
|
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 |
|
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: ***@***.***>
|
Rebverted in write lmabda:
// Apply minmax for standard temp
auto operation_state = id(${devicename}_operational_mode).state;
auto end_heating_unit = id(${devicename}_zone_2_end_heating_mode_emission_type).state;
auto end_cooling_unit = id(${devicename}_zone_2_end_cooling_mode_emission_type).state;
if (operation_state == "Cool") {
if (x < 5 and end_cooling_unit == "Fan Coil Unit") value_byte = 5;
if (x < 18 and end_cooling_unit != "Fan Coil Unit") value_byte = 18;
if (x >25) value_byte = 25;
} else if (operation_state == "Heat") {
if (x < 25 and end_heating_unit == "Underfloor Heating") value_byte = 25;
if (x > 55 and end_heating_unit == "Underfloor Heating") value_byte = 55;
if (x < 35 and end_heating_unit == "Radiator") value_byte = 35;
if (x > 60 and end_heating_unit == "Radiator") value_byte = 60;
}
Removed spaces in globals registers 210
|
@rysiulg I am a bit busy lately, this weekend I will look into your PR |
|
@rysiulg merged. Again thank you for your contribution ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.