Unclear Interpretation

[ramack]

Currently the field "Operating state" in "DeltaSol MX [heating circuit #]" is not interpreted, I currently get a 14 - whatever this means. Is there a chance to add a Enum type with meaningful names and a string representation for this field?

This potentially applies also to other controller specific packet fields.

[danielwippermann]

As far as I know 14 on the HC operating state corresponds to "Night mode".

Let me split the request into two parts:

"Is there a chance to add ... a string representation for this field?"

Yes, that would be possible, but since the textual representation cannot be extracted from the VSF it must be hard-coded (and localized) in the source of this project.

My naive solution would be to add a HashMap<Long, LocalizedText> to the PacketFieldSpec class and populate it for fields that are known to have enum-like functionality. Then formatTextValueFromRawValueInternal could first lookup a fixed textual representation from that HashMap (if populated) and fall back to formatting it the normal way for any other case. Fields would still keep their original type (e.g. "Number") and unit (e.g. "DegreesCelsius").

"Is there a chance to add a Enum type with meaningul names...?"

I don't 100% know what you mean by this. Do you have an example of such a Enum type in mind?

[ramack]

You suggested solution looks fine for me. - As long as I can query from the outside whether the field is a "plain" Number or a "InterpretableEnumNumer". The unit generally wouldn't make much sensor for those kind of fields, does it?

Second part is essentially the same as the first, just not using "only" strings - with the localization issue in resol-vbus-java, but using a datatype similar to SpecificationFile.UnitFamily, readable via something like PacketFieldValue.getEnumValue() for all the fields with this kind of data type? - But I did not think of whether this is feasible in a generic way... Maybe just using Strings is simpler an more generic?

[danielwippermann]

Well, the unit should be ignored in cases where a string representation is provided by the library. That way you could e.g. mark "888.8 °C" as "Defect", but all other sensor temperature values would just be the respective number plus the unit.

Regarding the second part: There are basically two different approaches to this problem:

1. Create a big enum for all possible variants found throughout the VBus spec, combining variants from different controllers and fields.

2. Just use Strings / intergers for those variants like the types in the VSF currently do.

The enum approach promises better type safety and perfomance in the short term. But since I hope that the enum variant information will one day be coming out of the VSF file, using an enum now means that code will break once that information comes out of the VSF at run-time instead of at compile-time.

So I would prefer to model something that could possibly be later replaced with a VSF-backed solution...

In any case I would propose to add two more APIs for that on each the PacketFieldSpec and PacketFieldValue classes:

1. getEnumVariantFromRawValue(...) would return the EnumVariant if the raw value corresponds to a enum variant (like in your case 14) or null if if does not (which will be the case for most fields). That method could be used to satisfy your request As long as I can query from the outside whether the field is a "plain" Number or a "InterpretableEnumNumer".

2. formatTextValueFromRawValue(..., Language lang) would be a new API extending the functionality of the existing formatTextValueFromRawValue without the language parameter. The variant including the language parameter would try to find a textual enum representation for the given language and default back to the formatting behaviour if no enum representation could be found.

Feedback is welcome...

[ramack]

So how can I detect an invalid value of 888.8 on API level without parsing the string? I don't like using the formatted strings. Except for enum values where it seem the best right now. Here I agree it is best to prepare for a VSF solution. But I also vote for backward compatibility such that in case a enum is newly added to be interpreted the the numeric value should be returned as before.

For the variant with and without language parameter I recommend to stick to the behaviour inexisting APIs with the defaulting to English in the parameterless variantbut otherwise do not change the behaviour. For a semantic difference with a different interpretation another name should ve chosen.

[danielwippermann]

So how can I detect an invalid value of 888.8 on API level without parsing the string?

Lets pretend the PacketFieldValue class has a getEnumVariant() method that returns an EnumVariant instance if the current raw value is associated with a special textual representation or null if not. Detecting a special value would mean you call that function and check whether the result is non-null.

For the variant with and without language parameter I recommend to stick to the behaviour inexisting APIs with the defaulting to English in the parameterless variantbut otherwise do not change the behaviour. For a semantic difference with a different interpretation another name should ve chosen.

I see your point and agree with it. Although I'd prefer to have an overloaded method with the same name instead of a different name... But I'll prototype around a bit and see what feels good...

[ramack]

Ah now I got what you mean. Looks good!

[danielwippermann]

I just pushed the commits e733466 and d38fe9f with a first proposal of the EnumVariant / Enum / getEnumVariantForValue / formatText support.

Commit e733466 adds the model classes to the SpecificationFile parent class. It also initializes some forged instances. This forging will be removed once the data comes out of the VSF file. The forged data is currently only handling the heating circuit operating state you initially requested since I had the respective list of possible values at hand.

Commit d38fe9f adds PacketFieldSpec#getEnumVariantForRawValue() as well as PacketFieldValue#getEnumVariant() and PacketFieldValue#formatText().

getEnumVariant basically does what I proposed above: it returns either a EnumVariant instance of it has one associated with the current raw value or otherwise returns null. This allows for a quick queck whether the current raw value has an associated textual respresentation.

formatText first tries to find a EnumVariant for a textual respresentation. If no EnumVariant is associated with the current raw value it will fall back to use formatTextValue instead.

Please give it a try. I appreciate your feedback on this! Thanks!

[ramack]

Did not yet try it, sorry. Nevertheless I have a question:

I feed the values I get into a typed system (OpenHAB), so for temperatures I want the values to be numerics, even in case the cable breaks, and there is no option to dynamically switch types between numeric and string for the error notification. For the OperationMode it is not necessary to have a numeric type at all, because none wants to calculate with the numbers.

In both situations I could use two output data channels one for the numeric value and one for the string representation, but can I know in advance whether there is a textual/enum interpretation for a specific field? - I expect to have some numeric values where interpretation is not meaningful for any value at all (e.g. SWVerison), for those I shuold not have the second, textual channel....

And it might even be interesting for the operation mode to use a "string-list" datatype. Can we extract all the enum/list types and their values (similar to the getUnits() I recently added)?

[danielwippermann]

In both situations I could use two output data channels one for the numeric value and one for the string representation, but can I know in advance whether there is a textual/enum interpretation for a specific field?

Currently not, but we would just have to add a getEnum wrapper to the PacketFieldSpec class. If you call that and it returns nullyou know that there is no special textual representation available.

Currently there is no way to flag a value to be enum-only. I consider it dangerous because in the past firmware updates tended to add new operating modes or other enum-variants that would then not be included in the enum list of the packet field until it is updated.

Can we extract all the enum/list types and their values (similar to the getUnits() I recently added)?

If we add the getEnum getter like described above you can then call getEnumVariants on the returned value to get a list of all supported enum variants.

I'll add the getEnum helper in the next couple of days.

[ramack]

That sound's great. But don't worry about the timing. I am really late in following your changes, so no hurry!

[danielwippermann]

I added the getEnum helper in a66ffff1de191bbc0e3410e573eb76ff67424a1f.

[ramack]

So the enum stuff for error flags and the operating mode in the MX is great. Thanks for that.

Where I still have some trouble is to take the decision whether a measurement value is a real value or not. E. g. broken sensor wires give 888.8, for some sensor values I have seen 999.9 both are not nice to be used in further evaluation, because calculating with those temperature values is not reasonable in most water-temperature environments and also if one sensor has a open load for one single measurement it is bad to have the 888.8 in the data, because it messes up every temperature graph.

For the pressure sensor channels in the MX it seems that 99.99 bar is the "invalid" value, so the question is whether there are more such values and how to detect them. Is there a API in resol-vbus-java, or do I really need to compare with 999.9 888.8 etc.? Esp. the 99.99 is critical, because I cannot generically ignore that because it is a feasible value at least for the solar collector sensor.

[danielwippermann]

Well, regarding the question whether there is an API for that: technically we could reuse the enum API for that case. It could return "Out of range" and "Unreachable" instead of "+/-888.8 °C" and "999.9 °C" respectively. By calling getEnumVariant() and judging its return value you could detect whether the sensor is in one of the special states.

Pressure and volume flow values could be handled the same way, only using different trigger values for their special state enum variants.

If that is okay for you I'll prototype such an approach and get that on a branch :)

[ramack]

This could work, but I do not yet see how I can generically differentiate the cases in the like heating circuit operation mode - where I in the meantime provide two values: the string representation which is nice for UserDisplay and a second numeric channel for usage in rules etc. Which should be independent from localization. If you use the same mechanism for 888.8 my code would do the same and have the string and number output, while the invalid numbers would still be there...

[danielwippermann]

The easiest way to differentiate it would be to look at the unit. The heating circuit operation mode is unit-less (None), but the sensor values are all equipped with their respective unit.

So a PacketFieldSpecwith getEnum() != null and getUnit() == unitNone would trigger your current behaviour of a raw number value and a string value representation.

And cases of getEnum() != null and getUnit() != unitNone could trigger a different mapping. I honestly don't know how to project that to openHAB's system of datapoints.

The simplest approach would be to provide both the raw number and a boolean whether that number is in one of its special states. But perhaps openHAB can combine that in the same datapoint (like returningNaN if in a special value state)?

[danielwippermann]

The discussion got stuck some time ago. Can this issue get closed or is there any work left to do?

[ramack]

No, I think we can close it for now. Thanks.