Skip to content

issue-91: Return the raw value instead of "None" in the __str__ method of the data fields#92

Merged
Bouni merged 4 commits intoBouni:mainfrom
Guzz-T:issue-91-selectionbase-unknown-values
May 1, 2023
Merged

issue-91: Return the raw value instead of "None" in the __str__ method of the data fields#92
Bouni merged 4 commits intoBouni:mainfrom
Guzz-T:issue-91-selectionbase-unknown-values

Conversation

@Guzz-T
Copy link
Copy Markdown
Contributor

@Guzz-T Guzz-T commented Mar 31, 2023

If the selection value is unknown, output the raw value instead of None to simplify the "reverse engineering".

fixes #91

Update:
Summary of the changes:

  • The data field class Base now stores the raw value instead of the already converted "user value"
  • Added a property Base.Raw to set and get the stored raw value directly
  • Added a property Base.Value to get the stored raw value converted tothe "user value"
  • Added a property Base.Value to set the stored value by a "user value" which is converted to raw data
  • The string representation Base.__str__ now output the raw value if the "user value" is None
  • The script dump-luxtronik.py now uses the string representation of the data field class

@kbabioch
Copy link
Copy Markdown
Collaborator

Is this only relevant for the from_heatpump function, or should it also be applied to to_heatpump?

@Guzz-T
Copy link
Copy Markdown
Contributor Author

Guzz-T commented Apr 1, 2023

In to heatpump value is a string, but an integer should be written to the heatpump. If we return the string, the interface may write the ASCII code of the first char? This could set the heatpump into an unexpected state. So we must not return the value there.

@kbabioch
Copy link
Copy Markdown
Collaborator

kbabioch commented Apr 1, 2023

In to heatpump value is a string, but an integer should be written to the heatpump. If we return the string, the interface may write the ASCII code of the first char? This could set the heatpump into an unexpected state. So we must not return the value there.

Ok, that makes sense.

However, I'm wondering: How is a normal API consumer (e.g. the Home Assistant integration) supposed to deal with from_heatpump with this change in place? Right now you will receive None when there is no mapping available. So you could easily do something like:

if from_heatpump(...):

With your change, you now have to check if a value from the options has been returned or some other integer, etc. This makes it less usable for an API consumer.

If I understand you correctly the actual go is to improve debugging / reverse engineering. Shouldn't we use some logging in that case? Unfortunately it seems like no logger is available, but this shouldn't be too hard.

@Guzz-T
Copy link
Copy Markdown
Contributor Author

Guzz-T commented Apr 1, 2023

I would wonder if homeassistant does anything other than print the raw value. To use the value in your example you have to write

If to_heatpump(...):
    Value = to_heatpump(...)

Better would be

Value = to_heatpump(...)
If Value:

But then you could use

If isinstance(Value, str):

to ensure that value is of type string

Another solution could be to implement a TryGet function, which outputs also a success flag.

Or

return str(value)

then the return value would always be of type string.

@Guzz-T Guzz-T force-pushed the issue-91-selectionbase-unknown-values branch from ea5e0a0 to 0b0ea95 Compare April 1, 2023 15:53
@kbabioch
Copy link
Copy Markdown
Collaborator

kbabioch commented Apr 5, 2023

Then let's go with the proposed str(...) solution. Personally I'm still not convinced that this is the right way (i.e. changing the API for the sake of debugging), but I can understand why this might be needed.

In any case, probably this will break some test cases?

@Guzz-T
Copy link
Copy Markdown
Contributor Author

Guzz-T commented Apr 5, 2023

I see you are not happy with these changes. I will try to solve this differently.

@Guzz-T Guzz-T force-pushed the issue-91-selectionbase-unknown-values branch 2 times, most recently from 4655bb1 to 4f9fe8e Compare April 5, 2023 21:01
@Guzz-T
Copy link
Copy Markdown
Contributor Author

Guzz-T commented Apr 5, 2023

Please checkout the new solution:
The raw data (_raw) is now stored instead of the already converted value (value). The conversions are now processed as needed. This should increase the performance, as usually only a small part of the >1000 data fields is used. In addition, we can also output the raw value afterwards (property raw). One use case is the new string representation _str_ which returns the raw value when the converted value is None

@Bouni
Copy link
Copy Markdown
Owner

Bouni commented Apr 13, 2023

@Guzz-T Can you rebase this please, I've just merged your multiple heatpumps PR #86

…nd. The string representation now outputs the raw value if the user value is 'None'.
@Guzz-T Guzz-T force-pushed the issue-91-selectionbase-unknown-values branch from 1d743f2 to 8e0f9be Compare April 13, 2023 19:28
@Guzz-T Guzz-T force-pushed the issue-91-selectionbase-unknown-values branch from 8e0f9be to 04c8e2b Compare April 13, 2023 19:31
@Guzz-T Guzz-T requested a review from kbabioch April 13, 2023 19:35
@Guzz-T
Copy link
Copy Markdown
Contributor Author

Guzz-T commented Apr 13, 2023

@Guzz-T Can you rebase this please, I've just merged your multiple heatpumps PR #86

Done

@Guzz-T Guzz-T changed the title issue-91: Return the raw value instead of None if the SelectionBase.from_heatpump value cannot be looked up issue-91: Return the raw value instead of "None" in the __str__ method of the data fields Apr 13, 2023
@Bouni Bouni merged commit 4f4ec32 into Bouni:main May 1, 2023
This was referenced May 1, 2023
@Guzz-T Guzz-T deleted the issue-91-selectionbase-unknown-values branch January 5, 2026 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SelectionBase suppresses the output of unknown values

3 participants