EmonCMS - 9.8.10 | 2017.08.17

[chaveiro]

Input null values fix.

[TrystanLea]

Not sure how I missed it but:

return ($lastvalue ? (float)$lastvalue : null);

changes inputs and feed values that are 0 to NULL in the input and feeds list..

[TrystanLea]

Could we also use if (condition) { } else { } notation, I find the ($lastvalue ? (float)$lastvalue : null); less clear to read..

[TrystanLea]

How about this as an alternative?

        if (!is_numeric($lastvalue["time"])) $lastvalue["time"] = null;
        if (is_nan($lastvalue["time"])) $lastvalue["time"] = null;
        $row['time'] = $lastvalue['time'];
        
        if (!is_numeric($lastvalue["value"])) $lastvalue["value"] = null;
        if (is_nan($lastvalue["value"])) $lastvalue["value"] = null;
        $row['value'] = $lastvalue['value'];

[chaveiro]

You're absolutely right, see my refactor.

[TrystanLea]

Hello @chaveiro , Is this pull request needed now? Your changes revert my changes? I see there are some other changes to, would it be good to separate this out?

[chaveiro]

Your changes on feed_model? They here refactored a check for === null is also needed.

[TrystanLea]

I dont quite understand why your replacing:

if (!is_numeric($lastvalue["time"])) $lastvalue["time"] = null;
if (is_nan($lastvalue["time"])) $lastvalue["time"] = null;

with:

if (!isset($lastvalue['time']) || $lastvalue['time'] === false) {
    $lastvalue['time'] = null;
} else {
    $lastvalue['time'] = (int) $lastvalue['time'];
}

your removing both my is_numeric check and is_nan check..?

[chaveiro]

The last value can be unset when redis is flushed and the check for false ensures we catch the null. By casting to int i ensure to return 0 if it was not a numeric for any other reason.

…

On Aug 6, 2017 08:20, "Trystan Lea" ***@***.***> wrote: I dont quite understand why your replacing: if (!is_numeric($lastvalue["time"])) $lastvalue["time"] = null; if (is_nan($lastvalue["time"])) $lastvalue["time"] = null; with: if (!isset($lastvalue['time']) || $lastvalue['time'] === false) { $lastvalue['time'] = null; } else { $lastvalue['time'] = (int) $lastvalue['time']; } your removing both my is_numeric check and is_nan check..? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#694 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEUUH4fMcv1zXnvJKnDvH_0LthhfBH33ks5sVWlGgaJpZM4OdbYF> .

[TrystanLea]

I've created a test script to try and have a consistent way of testing this and unfortunately (float) does not convert NAN to 0. My proposed solution does catch this case.

You could alternatively add the following check to fix?:

if (!isset($lastvalue['value']) || $lastvalue['value'] === false) {
    $lastvalue['value'] = null;
} else {
    $lastvalue['value'] = (float) $lastvalue['value'];
    if (is_nan($lastvalue["value"])) $lastvalue["value"] = null;
}

Test code

<?php

$test = array(
    array("time"=>123, "value"=>123),
    array("time"=>123, "value"=>45.6),
    array("time"=>123, "value"=>NAN),
    array("time"=>123, "value"=>null),
    array("time"=>123, "value"=>"string"),
    array("time"=>45.6, "value"=>123),
    array("time"=>NAN, "value"=>123),
    array("time"=>null, "value"=>123),
    array("time"=>"string", "value"=>123)
);
    
foreach ($test as $lastvalue) 
{
      
    /*
    // V1

    $lastvalue['time'] = $lastvalue['time'] * 1; 
    $lastvalue['time'] = (int) $lastvalue['time'];
    if (is_nan($lastvalue['time'])) $lastvalue['time'] = 0;

    $lastvalue['value'] = $lastvalue['value'] * 1; 
    $lastvalue['value'] = (float) $lastvalue['value'];
    if (is_nan($lastvalue['value'])) $lastvalue['value'] = 0;

    // V2

    if (!is_numeric($lastvalue["time"])) $lastvalue["time"] = null;
    if (is_nan($lastvalue["time"])) $lastvalue["time"] = null;

    if (!is_numeric($lastvalue["value"])) $lastvalue["value"] = null;
    if (is_nan($lastvalue["value"])) $lastvalue["value"] = null;


    // V3
    */
        
    if (!isset($lastvalue['time']) || $lastvalue['time'] === false) {
        $lastvalue['time'] = null;
    } else {
        $lastvalue['time'] = (int) $lastvalue['time'];
    }

    if (!isset($lastvalue['value']) || $lastvalue['value'] === false) {
        $lastvalue['value'] = null;
    } else {
        $lastvalue['value'] = (float) $lastvalue['value'];
    }

    
    print json_encode($lastvalue)."\n";

}

[chaveiro]

I've been testing with http://phptester.net/, on php 7.0 it's fine but on earlier versions the NaN is converted to -9223372036854775808?
It worth an additional check for this then, will make the change later.

[chaveiro]

Some fixes on last commit

[TrystanLea]

Hello @chaveiro

I tested your changes this morning, with the following test code:

<?php

$test = array(
    array("time"=>123, "value"=>123),
    array("time"=>123, "value"=>45.6),
    array("time"=>123, "value"=>NAN),
    array("time"=>123, "value"=>null),
    array("time"=>123, "value"=>"string"),
    array("time"=>45.6, "value"=>123),
    array("time"=>NAN, "value"=>123),
    array("time"=>null, "value"=>123),
    array("time"=>"string", "value"=>123)
);

foreach ($test as $lastvalue) 
{    
    if (!isset($lastvalue['time']) || $lastvalue['time'] === false || is_nan($lastvalue['time'])) {
        $lastvalue['time'] = null;
    } else {
        $lastvalue['time'] = (int) $lastvalue['time'];
    }

    if (!isset($lastvalue['value']) || $lastvalue['value'] === false || is_nan($lastvalue['value'])) {
        $lastvalue['value'] = null;
    } else {
        $lastvalue['value'] = (float) $lastvalue['value'];
    }

    print json_encode($lastvalue)."\n";
}

Im getting the following error, sorry!:

php test_decode.php
{"time":123,"value":123}
{"time":123,"value":45.6}
{"time":123,"value":null}
{"time":123,"value":null}
PHP Warning: is_nan() expects parameter 1 to be float, string given in /home/trystan/Desktop/test_decode.php on line 47
{"time":123,"value":0}
{"time":45,"value":123}
{"time":null,"value":123}
{"time":null,"value":123}
PHP Warning: is_nan() expects parameter 1 to be float, string given in /home/trystan/Desktop/test_decode.php on line 41
{"time":0,"value":123}

[chaveiro]

Ops, It should be better now...

Test code:

<?php
$test = array(
    array(),
    array("time"=>123, "value"=>123),
    array("time"=>false, "value"=>false),
    array("time"=>123, "value"=>45.6),
    array("time"=>123, "value"=>NAN),
    array("time"=>123, "value"=>null),
    array("time"=>123, "value"=>"string"),
    array("time"=>45.6, "value"=>123),
    array("time"=>NAN, "value"=>123),
    array("time"=>null, "value"=>123),
    array("time"=>"string", "value"=>123)
);

foreach ($test as $lastvalue) 
{    
	
    if (!isset($lastvalue['time']) || !is_numeric($lastvalue['time']) ) {
        $lastvalue['time'] = null;
    } else {
        $lastvalue['time'] = (int) $lastvalue['time'];
    }
	
    if (!isset($lastvalue['value']) || !is_numeric($lastvalue['value'])) {
        $lastvalue['value'] = null;
    } else {
        $lastvalue['value'] = (float) $lastvalue['value'];
    }

    print json_encode($lastvalue)."\n";
}

[TrystanLea]

Unfortunatly

array("time"=>123, "value"=>NAN),

still fails for me.

Is there any reason why we should not use my suggestion:

if (!is_numeric($lastvalue["time"])) $lastvalue["time"] = null;
if (is_nan($lastvalue["time"])) $lastvalue["time"] = null;

if (!is_numeric($lastvalue["value"])) $lastvalue["value"] = null;
if (is_nan($lastvalue["value"])) $lastvalue["value"] = null;

or my proposed modification of your suggestion?

if (!isset($lastvalue['time']) || $lastvalue['time'] === false) {
    $lastvalue['time'] = null;
} else {
    $lastvalue['time'] = (int) $lastvalue['time'];
    if (is_nan($lastvalue["time"])) $lastvalue["time"] = null;
}

if (!isset($lastvalue['value']) || $lastvalue['value'] === false) {
    $lastvalue['value'] = null;
} else {
    $lastvalue['value'] = (float) $lastvalue['value'];
    if (is_nan($lastvalue["value"])) $lastvalue["value"] = null;
}

[TrystanLea]

both of which work fine! although there is a slight difference in output:

my initial suggestion converts NAN and "string" to null, while yours with my modification converts NAN to null and "string" to 0

[chaveiro]

I see, different versions of php get different results...
I've added is_nan as you had originally. Also !isset check is required.
Was trying to limit the if conditions as they will slow processing speed.

[TrystanLea]

Great it passes! :) I will merge this.

Is there any risk in removing the float casting in input_model set_timevalue?
The input methods cast it anyway I guess? but is there any harm doing it again?

[chaveiro]

You're referring to this line ?
https://github.com/chaveiro/emoncms/blob/84ab0602e5aedd305a788ef776ad5ece86ddedd5/Modules/input/input_model.php#L89
-> $value = $value; // Dont cast

The $stmt->bind_param casts already (to double).
By not casting in PHP will allow the mysqli->prepare statement to save nulls on the database.

[TrystanLea]

Thanks @chaveiro that sounds fine