Autor Thema: small improvement ReadingsVal  (Gelesen 1213 mal)

Offline immi

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1004
small improvement ReadingsVal
« am: 05 August 2020, 19:05:13 »
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
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


Offline rudolfkoenig

  • Administrator
  • Hero Member
  • *****
  • Beiträge: 22806
Antw:small improvement ReadingsVal
« Antwort #1 am: 05 August 2020, 20:27:36 »
As this function is widely used: is somebody there opposing the change?

Offline DS_Starter

  • Developer
  • Hero Member
  • ****
  • Beiträge: 5862
Antw:small improvement ReadingsVal
« Antwort #2 am: 05 August 2020, 23:00:33 »
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 ?
ESXi 6.5 @NUC6i5SYH mit FHEM auf Debian 10, DbLog/DbRep mit MariaDB auf Synology 415+
Maintainer: SSCam, SSChatBot, SSCal, DbLog/DbRep, Log2Syslog, SMAPortal, Watches, Dashboard
Kaffeekasse: https://www.paypal.me/HMaaz
Contrib: https://svn.fhem.de/trac/browser/trunk/fhem/contrib/DS_Starter

Offline herrmannj

  • Global Moderator
  • Hero Member
  • ****
  • Beiträge: 5652
Antw:small improvement ReadingsVal
« Antwort #3 am: 05 August 2020, 23:03:32 »
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.
smartVisu mit fronthem, einiges an HM, RFXTRX, Oregon, CUL, Homeeasy, ganz viele LED + Diverse
Zustimmung Zustimmung x 3 Liste anzeigen

Offline betateilchen

  • Developer
  • Hero Member
  • ****
  • Beiträge: 16517
  • s/fhem\.cfg/configDB/g
Antw:small improvement ReadingsVal
« Antwort #4 am: 06 August 2020, 10:31:03 »
it can be easily solved with user-code. 

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

I totally agree.
-----------------------
Unaufgeforderte Anfragen per email werden von mir nicht beantwortet. Dafür ist das Forum da.
-----------------------
Lesen gefährdet die Unwissenheit!

Offline immi

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1004
Antw:small improvement ReadingsVal
« Antwort #5 am: 06 August 2020, 18:40:51 »
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

Zitat
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.
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))}

Zitat
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

Offline Dr. Boris Neubert

  • Global Moderator
  • Hero Member
  • ****
  • Beiträge: 4768
  • Are we just self-replicating DNA?
Antw:small improvement ReadingsVal
« Antwort #6 am: 07 August 2020, 11:17:32 »
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

Globaler Moderator, Developer, aktives Mitglied des FHEM e.V. (Marketing, Verwaltung)
Bitte keine unaufgeforderten privaten Nachrichten!
Zustimmung Zustimmung x 2 Liste anzeigen

Offline betateilchen

  • Developer
  • Hero Member
  • ****
  • Beiträge: 16517
  • s/fhem\.cfg/configDB/g
Antw:small improvement ReadingsVal
« Antwort #7 am: 07 August 2020, 19:07:22 »
@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.
-----------------------
Unaufgeforderte Anfragen per email werden von mir nicht beantwortet. Dafür ist das Forum da.
-----------------------
Lesen gefährdet die Unwissenheit!

Offline Dr. Boris Neubert

  • Global Moderator
  • Hero Member
  • ****
  • Beiträge: 4768
  • Are we just self-replicating DNA?
Antw:small improvement ReadingsVal
« Antwort #8 am: 07 August 2020, 19:25:06 »
(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.
Globaler Moderator, Developer, aktives Mitglied des FHEM e.V. (Marketing, Verwaltung)
Bitte keine unaufgeforderten privaten Nachrichten!

Offline Beta-User

  • Developer
  • Hero Member
  • ****
  • Beiträge: 11611
  • eigentlich eher "user" wie "developer"
Antw:small improvement ReadingsVal
« Antwort #9 am: 07 August 2020, 19:31:41 »
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).
Server: HP-T620@Debian 10, aktuelles FHEM + ConfigDB | CUL_HM (VCCU) | MQTT2: MiLight@ESP-GW | MySensors: seriell, v.a. 2.3.1@RS485 | ZWave | ZigBee@deCONZ | SIGNALduino | MapleCUN | BT@OpenMQTTGateway
svn:MySensors, WeekdayTimer, RandomTimer, Twilight,  AttrTemplate => {mqtt2, mysensors, zwave}

Offline betateilchen

  • Developer
  • Hero Member
  • ****
  • Beiträge: 16517
  • s/fhem\.cfg/configDB/g
Antw:small improvement ReadingsVal
« Antwort #10 am: 07 August 2020, 19:41:01 »
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.
-----------------------
Unaufgeforderte Anfragen per email werden von mir nicht beantwortet. Dafür ist das Forum da.
-----------------------
Lesen gefährdet die Unwissenheit!

Offline amenomade

  • Developer
  • Hero Member
  • ****
  • Beiträge: 6573
Antw:small improvement ReadingsVal
« Antwort #11 am: 07 August 2020, 19:58:45 »
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 ?
FHEM 5.9 Pi 3, EchoDot, CUL868+Selbstbau 1/2λ-Dipol-Antenne, USB Optolink / Vitotronic, Debmatic und HM / HmIP Komponenten, Rademacher Duofern Jalousien, Fritz!Dect Thermostaten, Proteus

Offline Christoph Morrison

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1242
Antw:small improvement ReadingsVal
« Antwort #12 am: 08 August 2020, 15:58:43 »
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.).

Zitat
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.

Offline Dr. Boris Neubert

  • Global Moderator
  • Hero Member
  • ****
  • Beiträge: 4768
  • Are we just self-replicating DNA?
Antw:small improvement ReadingsVal
« Antwort #13 am: 08 August 2020, 16:12:44 »
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.
Globaler Moderator, Developer, aktives Mitglied des FHEM e.V. (Marketing, Verwaltung)
Bitte keine unaufgeforderten privaten Nachrichten!

Offline Christoph Morrison

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1242
Antw:small improvement ReadingsVal
« Antwort #14 am: 08 August 2020, 16:53:34 »
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.

Offline Dr. Boris Neubert

  • Global Moderator
  • Hero Member
  • ****
  • Beiträge: 4768
  • Are we just self-replicating DNA?
Antw:small improvement ReadingsVal
« Antwort #15 am: 08 August 2020, 17:03:56 »
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.

Agreed.
Globaler Moderator, Developer, aktives Mitglied des FHEM e.V. (Marketing, Verwaltung)
Bitte keine unaufgeforderten privaten Nachrichten!

Offline immi

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1004
Antw:small improvement ReadingsVal
« Antwort #16 am: 09 August 2020, 08:51:11 »
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.
Dear Boris
I implemented the readings of 00_THZ as transparent representation of heatpump registers logic (oxymoron :) ....) .
Some registers contain 1 value, some 50+ values.
We reverse-engineered about 230 registers (depending on the firmware and the build year), many of them undocumented.
Many years ago >7, after long discussions, we decided not to flatten (unroll) the registers with multiple values, in order to keep the number of readings low (230, instead of nearly 1000 if flattened)
Plotting was never a problem (you can address multiple columns easily) and userreadings made advanced users happy.

You convinced me that the "use case" is only related to thz; Maybe I will write a helper function inside the thz module, which will not affect other modules.


Dear Christoph
you asked for some register examples; please find a screenshot enclosed

immi

Offline Dr. Boris Neubert

  • Global Moderator
  • Hero Member
  • ****
  • Beiträge: 4768
  • Are we just self-replicating DNA?
Antw:small improvement ReadingsVal
« Antwort #17 am: 09 August 2020, 10:26:07 »

Dear Immi,

I implemented the readings of 00_THZ as transparent representation of heatpump registers logic (oxymoron :) ....) .
Some registers contain 1 value, some 50+ values.
We reverse-engineered about 230 registers (depending on the firmware and the build year), many of them undocumented.
Many years ago >7, after long discussions, we decided not to flatten (unroll) the registers with multiple values, in order to keep the number of readings low (230, instead of nearly 1000 if flattened)

thank you for this additional information. Now I much better understand the actual case at hand.

Given that you are thinking about including a helper function into the THZ module, I suppose that this topic has come to a resolution.

You probably had good reasons for choosing the register-based implementation over the readings-based implementation. I suggest not to reopen this discussion, at least not in this thread.

Kind regards
Boris
Globaler Moderator, Developer, aktives Mitglied des FHEM e.V. (Marketing, Verwaltung)
Bitte keine unaufgeforderten privaten Nachrichten!

Offline Christoph Morrison

  • Developer
  • Hero Member
  • ****
  • Beiträge: 1242
Antw:small improvement ReadingsVal
« Antwort #18 am: 09 August 2020, 23:58:04 »
So the "solution" is THZ_Val(), mostly a copy of ReadingsVal() instead of transforming sControl into a more accessible data structure (e.g. JSON) and adding a retrieval function if necessary - and THZ_Val() does also reside in the main namespace (and clutters common ground).

 

decade-submarginal