Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Maintain a list of changed parameter/calculation names #168

Open
gerw opened this issue Feb 21, 2024 · 3 comments · May be fixed by #187 or #191
Open

Maintain a list of changed parameter/calculation names #168

gerw opened this issue Feb 21, 2024 · 3 comments · May be fixed by #187 or #191
Milestone
New pypi release ...

Comments

@gerw
Copy link
Collaborator

gerw commented Feb 21, 2024

Guzz-T noted in #132 that we have changed some names since 0.3.14, in particular, Unknown_Calculation_239 has been changed to VBO_Temp_Spread_Soll. I suspect that this will also happens many times in the future.

Users of our library will find that Unknown_Calculation_239 will no longer work after an update and in some occasions, it might be quite cumbersome to find out the new name. (For Unknown_Calculation_239 it is pretty obvious, that this is calculation 239, but in other cases this might not be true).

I think that we should maintain a list of changes of these names, i.e., something like

changed_names = {
    "Unknown_Calculation_239" : "VBO_Temp_Spread_Soll"
}

in constants.py. Then, DataVector._lookup could also resolve old names (and maybe spit out a warning that users can change their code?).

  • If we do so, we should also set up a unit test, which assures that all old names can be resolved.
  • There seems to be one limit to the approach: We can't resolve the situation where "ID_BarFoo" is mapped to parameter 42 in 0.3.14 but is mapped to 23 in 0.3.15. However, this does not seem to be a big deal, since we can (almost) freely choose the parameter names.
  • What about ID_WEB_SoftStand? This old calculation will be replaced by a higher level function. I think that an access of this old name should result in a meaningful error message. Maybe we can add a dictionary obsolete_names which contains these error messages, e.g.,
obsolete_names = {
    "ID_Web_SoftStand" : 'This parameter has be replaced by the function "get_firmware_version()" of LuxtronikData / Luxtronik.'
}
@gerw gerw added this to the New pypi release 0.3.15 milestone Feb 21, 2024
gerw added a commit to gerw/python-luxtronik that referenced this issue Feb 23, 2024
@gerw
Add compatibility feature
34d1617 
Closes Bouni#168
@gerw gerw mentioned this issue Feb 23, 2024
Open
@Guzz-T
Copy link
Contributor

Guzz-T commented Feb 24, 2024

If we still support old names, do we also have to support older luxtronik firmware versions? For example the used-energy-value is only available within the (current) latest firmware version. But not in previous ones.

@gerw
Copy link
Collaborator Author

gerw commented Feb 24, 2024

This issue is about compatibility of code that works with older library versions. For example, in 0.3.14 you could have

l.calculations.get("Unknown_Calculation_239")

and, as you observed, this no longer works with the present state of the library. This is totally unrelated to the firmware version of your luxtronik controller. I think that, ideally, code that works with old versions of the library still works with the newer versions. And if this is not possible, we should raise a meaningful error message.

I do not see any compatibility issues w.r.t. different firmware versions. Of course, some parameters/calculations are only available in certain versions, e.g., the calculation 257: Power("Heat_Output"), is only available in newer versions, while the support of the "MainMenuStatusLine" in calculations 117-119 was dropped a long time ago. I do not know any instance where the meaning of some entry changed its meaning, but there are a couple of instances, where a parameter/calculation is no longer active, i.e., it is always 0 (or we even see some noise) and writing to it doesn't change anything (besides the value itself). I assume that alpha innotec wants to avoid this, since that would lead to different complications related to their own software (the app, I think).

@Guzz-T
Copy link
Contributor

Guzz-T commented Feb 24, 2024

I am aware that this question was a bit "off-topic". However, the library version is usually connected to the firmware version.

It was also more of a question whether it is worth creating a new issue instead of a request to change this here.

@Guzz-T Guzz-T linked a pull request Oct 31, 2024 that will close this issue
Open
@Guzz-T Guzz-T linked a pull request Nov 20, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
New pypi release 0.3.15
2 participants
@gerw @Guzz-T