Rounding error when storing value in dummy or rules variable

[Frank-Huber]

Hi there,

I have an issue with Rules in build mega-20190311,

I have the following small rule: `on PN532#ChipID do TaskValueSet 3,2,%eventvalue% TaskValueSet 3,1,1 timerSet,1,1 endon

on Rules#Timer=1 do TaskValueSet 3,1,0 TaskValueSet 3,2,0 endon`

PN532 is a "RFID - PN532", Task 3 is a dummy. when ever I batch a RFID card teh dummy should get the RFID number. but in real the number is manipulated for no reason. I also tried TaskValueSet 3,2,[RFID#Tag] which has the exact same result

a tag with 2610951181 will get 2610951168 in the dummy. a tag with 72772698 will get 72772696 in the dummy. a tag with 74467418 will get 74467416 in the dummy. a tag with 75985498 will get 75985496 in the dummy. a tag with 3071250732 will get 3071250688 in the dummy.

Is there any reason for this number manipulating? Am I doing anything wrong?

Sorry if I have forgotten any important information.

This is my first post on GitHub.

Thanks & br Frank

[TD-er]

I can imagine it has to do with float to int conversion. The RFID reader plugins do not output a float value, but do create a 32 bit unsigned int in a different way. I guess the TaskValueSet command does collect the values as float and process it as float, while it should not.

I am not really sure how this can be changed, or even where it has to be changed.

[uzi18]

why you need to copy this value to dummy?

[Frank-Huber]

Hi, good morning. I use this mechanism to detect if the tag is removed.

The rule sets the dummy to the Tag ID, starts a timer for 1 sec and sets the "presence" (also in the Dummy) to 1. As long as teh Tag is on the reader there is a event from it every second, so the timer never stops as it is re-triggered always. If the Tag is removed after a second the timer ends which will then set the Dummy TagID and presence to "0" I do only transmit the Dummy to my Home Automation.

Thanks & br Frank

[TD-er]

Hmm, storing it to the dummy may always introduce the same rounding error (you're loosing +/- 10 bits from unsigned int to float) so you can still use that to keep track of it. N.B. You can also use the relatively new variables, for example see Read the Docs - Averaging Filter But I would suggest to use the value from the plugin itself to send it to the home automation. Grabbing the value from the plugin (using [name#variable] syntax) should grab it like it is meant to be. So a float should have the set number of decimals and an integer value is an integer.

[Frank-Huber]

Thanx TD-er, will have a look into the doc. It is always the same manipulation, so I can simply use the wrong number there.

In the beginning I had it as [name#variable] which resulted in the same problem. then I tried %eventvalue%

topic is I have to recognize the removal of the Tag. so maybe I try a mix, TagID direct and presence by the Dummy.

FYI, this is not for access control on a door, I use teh reader to build a "tunibox" Mediaplayer for the Kids. the HA controls a Kodi MediaPlayer behind.

br Frank

[TD-er]

You have to make sure the numbers you use are not sequential, or else they will be truncated into the same float value. And as soon as you're storing it in the dummy variable, then it will be converted to float

[uzi18]

maybe plugin should generate events like changed or removed...

[Frank-Huber]

Well, I have a value that should be transmitted to a dummy. If this is float or whatever should not care for the user. it simply should work. So this is a bug which should be fixed somewhen. For me there is no urgency, as I for now simply use the modified numbers.

the additional events of the plugin would be a great enhancement indeed. :-)

Will also try the "new" variables. maybe that works as it should. I am on travel two days now, so will test on weekend.

Thanks both for the help so far!

[uzi18]

Simple fix is to give access to high and low part of long so they will be easy to copy/compare

It is not a bug it is design limitations/feature request.

Bug is here: dummy declared as LONG and you can't copy cardid (it is also LONG) into this dummy.

[TD-er]

If the numbers you use are close to each other, you may also subtract the value of one of them. This will make the number of significant decimals a lot smaller, so then you do not have these rounding errors in the stored float value.

For example if your RFID values are something like 2345678901 ... 2345678910 then you can subtract 2345678900 from it and you're left with only 2 decimals to store. But that will only work when the values you try to process are close to each other. When merging ranges from different branches or batches the values may differ a lot and then this trick will not work.

[Frank-Huber]

uzi18, I am not a developer, so I did not understand a word. :-)

TD-er, unfortunately the numbers are various. but as told, no problem as the manipulation is static. so I use the modified values in my HA for now. this works since weeks. I will also try what happens with "old" Mifare tags with 4byte UID. if it's a 32bit topic they should keep correct.

[uzi18]

So what is your action for kodi if new card is detected?

[Frank-Huber]

example: Tag 1111 will start Radio Station xy Tag 2222 will start a Audio book Tag 3333 will start a TV series Tag removed will stop

I have all Tags directly assigned to the Kodi action. Unknown Tags will do nothing in the beginning New tags get programmed in the Logic first.

[TD-er]

Just pay attention when adding new cards, since they may result in the same float value.

[florenciob]

hi, I have the same problem. I'm using the rfir as a lock and storing into dummy variables the keys with access. so it the rfid read is within the keys into dummy variables 11 o 12, then acces is granted. but it doesnt work.

[TD-er]

I changed the topic title to make it more descriptive of the real issue

[uzi18]

@florenciob please post here your rfid task configuration screenshot

[florenciob]

The images are the devices

this is the rule On System#Boot Do GPIO,16,0 TaskValueSet 2, 1, 0 EndOn

On Lock#Tag Do

TaskValueSet 3, 1, [Lock#tag]

If [Lock#tag] = 944900659 If [relee#relee] =0 GPIO,16,1 TaskValueSet 2, 1, 1 Else GPIO,16,0 TaskValueSet 2, 1, 0 EndIf EndIf Delay 3000 EndOn

[uzi18]

@florenciob is it possible to you to paste here log from time when tag is readed and rules are executed? if you can paste it for 2 different rfids it will be better.

[florenciob]

Hi, here is the log. please let me know if there is anything else I could do.

90223880: WD : Uptime 1504 ConnectFailures 1958 FreeMem 17840 WiFiStatus 3 90232229: PN532: Tag: 944900659 20 90232264: EVENT: Lock#Tag=944900659 90232281: ACT : TaskValueSet 3, 1, 944900659 90232290: Command: taskvalueset 90232352: ACT : GPIO,16,0 90232356: SW : GPIO 16 Set to 0 90232359: ACT : TaskValueSet 2, 1, 0 90232368: Command: taskvalueset 90232377: ACT : Delay 3000 90232384: Command: delay 90245432: EVENT: Clock#Time=Tue,20:56 90249687: Dummy: value 1: 944900672.00 90249688: Dummy: value 2: 0.00

[uzi18]

@TD-er what do you think about it? :

diff --git a/src/Commands/Tasks.h b/src/Commands/Tasks.h
index d4a34234..0124fb6b 100644
--- a/src/Commands/Tasks.h
+++ b/src/Commands/Tasks.h
@@ -20,9 +20,18 @@ String Command_Task_ValueSet(struct EventStruct *event, const char* Line)
 {
        String TmpStr1;
        if (GetArgv(Line, TmpStr1, 4)) {
-               float result = 0;
-               Calculate(TmpStr1.c_str(), &result);
-               UserVar[(VARS_PER_TASK * (event->Par1 - 1)) + event->Par2 - 1] = result;
+               byte DeviceIndex = getDeviceIndex(Settings.TaskDeviceNumber[event->Par1 - 1]);
+               if (Device[DeviceIndex].VType == SENSOR_TYPE_LONG) {
+                       // TODO: Calculate result for value type LONG
+                       unsigned long val = (unsigned long) TmpStr1.toInt();
+                       int uservarindex = (VARS_PER_TASK * (event->Par1 - 1)) + event->Par2 - 1;
+            UserVar[uservarindex]    = (val & 0xFFFF);
+            UserVar[uservarindex +1] = ((val >> 16) & 0xFFFF);
+               } else {
+                       float result = 0;
+                       Calculate(TmpStr1.c_str(), &result);
+                       UserVar[(VARS_PER_TASK * (event->Par1 - 1)) + event->Par2 - 1] = result;
+               }
        }else  {
                //TODO: Get Task description and var name
                serialPrintln(String(UserVar[(VARS_PER_TASK * (event->Par1 - 1)) + event->Par2 - 1]));

we need also compare long values fix here

[uzi18]

@TD-er we can try to switch Calculate and rest of folks, to double insted of float, but this will give us more overhead.

[TD-er]

A double will take literally double the amount of memory. (is it stored somewhere?) And it is no guaranteed fix for "strange issues". For example comparing double values will also lead to unexpected results if you're trying to match them like you would do with ints.

I think we must first determine why a dummy plugin is used and not one of the new variables in the rules. If a variable is also usable, then we can decide to have long typed variables also. Those will be useful for other use cases too. Adding complexity to the dummy plugin is not my preferred way to fix it. With the fix you suggest, you'll split a value over 2 values in the dummy plugin. But how would you create a long again? Also any other plugin supporting long types values can only handle a single long value.

And last question... the RFID plugins output long values. Is that enough bits for all types of RFID?

[uzi18]

we always use for long 2 floats to store 32bits, just check function to format long value before send by controller

[uzi18]

@TD-er but we can use 3-4 floats instead to store 48/64 bits with long, this should be easy to extend

[TD-er]

In theory, we could also store a long in the memory space of a float. You then either have to do a reinterpret_cast, or use a struct with a union. So then we can store a long using the same amount of memory. We just have to make sure to properly keep track of the type. For normal plugins the type is fixed, but for a dummy (or a variable in the rules) it may change. This also means you have to set the value to 0 when changing the type, or else you may run into a crash since not all possible combinations of a long represent a valid float.

[uzi18]

@TD-er I mean only for Calculate to use double precission instead float as result. For Long we can leave it as is, eventually extend it to long long (64bits) it will resolve some problems with rfid cards.

This is how we use it: https://github.com/letscontrolit/ESPEasy/blob/086d5dff5c6e3346b038fa857237feddc6a5d6b1/src/StringConverter.ino#L188-L190

[TD-er]

The UserVar struct is also stored, to RTC memory. So those cannot be changed without breaking stuff.

[TD-er]

This is no longer an issue, or at least it is much less of an issue now. You can now store values in rule variables which are now of type double. Also the handling of floating point values is now much less prone to rounding errors when comparing values in rules.

Storing values with double resolution is not supported for dummy task values and will also not be supported there as it will break a lot of other code.