small improvement ReadingsVal

[immi]

dear Rudolf
in some cases the readings have more than one value (many columns)
e.g. deviceX:readingX        5 abc 10  15

would you mind adding an optional parameter to ReadingsVal which selects the column in the reading?
e.g.  ReadingsVal("deviceX","readingX",0,2)  —> 10

possible implementation

Code Auswählen Erweitern

sub
ReadingsVal($$$;$)
{
  my ($d,$n,$default,$col) = @_;
  if(defined($defs{$d}) &&
     defined($defs{$d}{READINGS}) &&
     defined($defs{$d}{READINGS}{$n}) &&
     defined($defs{$d}{READINGS}{$n}{VAL})) {
        my $tmp=$defs{$d}{READINGS}{$n}{VAL};
        if (defined($col) && defined((split ' ',$tmp)[$col]))  {return((split ' ',$tmp)[$col]);}
        else                                  {return $tmp;}
  }
  return $default;
}


this change could improve code readability especially in complex userreadings, when the user want to address only part of the reading
thanks
immi

[rudolfkoenig]

As this function is widely used: is somebody there opposing the change?

[DS_Starter]

Mir fällt momentan kein Gegenargument ein, möchte aber noch ergänzen dass diese Änderung meiner Meinung nach  konsequenterweise auch in ReadingsNum erfolgen müsste um z.B. Folgen der Art "5°C 7kmh 70pa" auch zu behandeln bzw. nur die Ziffern des addressierten Elements zu liefern, oder ?

sorry in english ...
I can't think of any counter-argument at the moment, but would like to add that in my opinion this change should also be made in ReadingsNum in order to treat consequences of the type "5°C 7kmh 70pa" or to provide only the digits of the addressed element, or ?

[herrmannj]

I'm not convinced. This is a very exotic application and readings have no column function by default. In the rare cases where you may need it, it can be easily solved with user-code. 

It will add confusion and inconsistency without delivering a significant amount of value.

[betateilchen]

it can be easily solved with user-code. 

It will add confusion and inconsistency without delivering a significant amount of value.

I totally agree.

[immi]

Dear deveopers,
thanks for considering my small proposal; I try to reply to your concerns.

Mir fällt momentan kein Gegenargument ein, möchte aber noch ergänzen dass diese Änderung meiner Meinung nach  konsequenterweise auch in ReadingsNum

agree

This is a very exotic application and readings have no column function by default.

yes and no; readings with column are supported in fhem since long time;
e.g. have a look to the column function in svg-plots

In the rare cases where you may need it, it can be easily solved with user-code. 

completely agree with you; but some solutions are prone to userfailures
e.g. look at a tipical config files from my users; I want to simplify their user-codes.

Code Auswählen Erweitern


attr Mythz userReadings Rel_humidity:sGlobal.* {((split ' ',ReadingsVal("Mythz","sGlobal",0))[67]) + 11.5}, flow_temp:sGlobal.* {((split ' ',ReadingsVal("Mythz","sGlobal",0))[3])}, return_temp:sGlobal.* {((split ' ',ReadingsVal("Mythz","sGlobal",0))[5])}, outside_temp:sGlobal.* {((split ' ',ReadingsVal("Mythz","sGlobal",0))[1])}, dhw_temp:sGlobal.* {((split ' ',ReadingsVal("Mythz","sGlobal",0))[9])}, inside_temp:sHC1.* {((split ' ',ReadingsVal("Mythz","sHC1",0))[27])}, CopHC:sHeatRecoveredDay.* {sprintf("%.2f", ReadingsNum("Mythz","sHeatHCDay",1) / ReadingsNum("Mythz","sElectrHCDay",1))}, CopDHW:sHeatRecoveredDay.* {sprintf("%.2f", ReadingsNum("Mythz","sHeatDHWDay",1) / ReadingsNum("Mythz","sElectrDHWDay",1))}


It will add confusion and inconsistency without delivering a significant amount of value.

Most of Fhem users have never seen ReadingsVal.
The change is completely backwardcompatible: experienced users will have no problem with an optional parameter.
Newbe could have a real benefit.
Personally, I have no advantage from the change. Not asking that for me.
p.s. you can write in german;I understand it; I am just too lazy, therefore I use eng.

b.r.
immi

[Dr. Boris Neubert]

I oppose the actual proposal as it adds a singular aspect to the functions which may lead to more and more such requests, ending up in a disparate overall picture. We have a similar long-standing request regarding number precision in the time-series function with I oppose for the same reasons (and because I would like to separate calculation from presentation).

Though, I see the need from the perspective of the user to greatly simplify the user code.

My suggestion is to amend MyUtils with a set of helper functions that address specific needs. We have such helper functions included in the modules as well, for example the WeatherToHtml() function. That keeps the module code clean from functions dedicated to particular use cases but simplifies the user's life.

Make it a contest and ask for contributions, I would say. Some code review and then inclusion into MyUtils.pm.

Best regards,
Boris

[betateilchen]

@immi:

You can design two functions "ReadingsValArray()" and "ReadingsNumArray()" and ask Rudi for adding the functions into 99_Utils.pm.
Please think about another additional parameter "separator" as there are reading values separated by ";" or "|" or other characters.

In my opinion this way wouldn't really hurt anyone.

[Dr. Boris Neubert]

(OT: I've gotten a bit rusty lately, actually it's 99_Utils.pm, not MyUtils.pm, as betateilchen seconded).

For me this is a totally agreeable solution.

[Beta-User]

I totally agree with @betateilchen.

May I suggest a slightly different naming: ReadingsValPARTx() etc.. Imo. this would be more in line with $EVTPARTx in a lot of event handlers. (I'm aware, in the mentionned context PARTx always refers to space as separator, so this could be different here, but on the other hand, it's a feature for advanced users).

[betateilchen]

May I suggest a slightly different naming:

PART in EVTPARTx is always used with the "number" passed in its name. ReadingsValArray() should be designed to use the element number as a parameter.

Please do not mess up different behaviors by similar/misleading naming.

[amenomade]

PART in EVTPARTx is always used with the "number" passed in its name. ReadingsValArray() should be designed to use the element number as a parameter.

Please do not mess up different behaviors by similar/misleading naming.

On the other hand the name "ReadingsValArray" suggests (at least for me) that the function is returning an array. If the function is to return the element corresponding to the parameter passed, wouldn't it be better with something like ReadingsValPart or if misleading ReadingsValElmt / Item ?

[Christoph Morrison]

Before changing an API, please show the list for the device Mythz. I quite don't like the idea of wrapping FHEM code over Perl code again (and again, and again, etc.).

Code Auswählen Erweitern

Rel_humidity:sGlobal.* {((split ' ',ReadingsVal("Mythz","sGlobal",0))[67]) + 11.5},

Really, at least 68 columns with values in a single reading?

Readings are a very flat, key-value data structure. Enhancing them by storing space delimited data is IMHO some kind of misusage / inadvertent usage. We probably do need a more sophisticated option for storing (user) data.

[Dr. Boris Neubert]

Really, at least 68 columns with values in a single reading?

Good point. If such complexity is required to get to a reading in the first place, I would rather recommend rethinking the code of the device module and create separate FHEM readings from the single data line received from the physical device.

Though, this thus not void the original idea and its seemingly agreeable solution.

[Christoph Morrison]

Though, this thus not void the original idea and its seemingly agreeable solution.

This does not invalidate the idea, but the other things i mention are doing this. It's a cumbersome way to store any structured data and should not be supported. We do have plenty of methods for structuring data in a single value string (JSON, XML, Storables, etc.) and those values can be unrolled into single key-value pairs as you mentioned.

But not any of these do justify a change to the API.