Weather-Station-Software / live-weather-station

Display on your WordPress site, in many elegant ways, the meteorological data collected by public or personal weather stations.
https://weather.station.software/
GNU General Public License v2.0
8 stars 3 forks source link

Ambient Weather - Incorrect assignment of temperature data from extra modules to outdoor temperature #22

Open dbusch78 opened 1 year ago

dbusch78 commented 1 year ago

Hello there,

Congratulations on taking over the project. I really appreciate the ability to display my weather station's data on my website. I was pleased to discover that you are on GitHub, providing a platform for users to contribute and report bugs. I'm in Information Security as one of my Jobs so I hope this is complete and helps identify the issues.

Description:

I've noticed an issue in the file includes/traits/AmbientPluginBaseClient.php. The temperature data from extra modules ('temp' . $i . 'f') is being assigned to the 'temperature' key in the $device array. This seems to be overwriting the outdoor temperature data, resulting in the temperature from extra modules being displayed as the outdoor temperature.

I am using the Ambient Weather Station model WS-5000. However, based on a brief review of the API documentation from Ambient Weather, it appears that the API output might not be dependent on the model. Ambient Weather seems to have standardized the outputs and simply omits data if it doesn't exist for a particular model. I've only had this weather station for a few weeks.

Steps to reproduce:

  1. Add the station to the WordPress plugin.
  2. Wait for the API to sync.
  3. Verify that the outdoor temperature displayed is incorrect.
  4. Remove the station from the WordPress plugin.
  5. Enable debug logs.
  6. Re-add the station to the WordPress plugin.
  7. Wait for the API to sync again.
  8. Verify that the outdoor temperature displayed is still incorrect.
  9. Go to the debug logs and retrieve the API log.
  10. In the API log, find the correct outdoor temperature field and the additional temperature and humidity sensor (in this case, a Pool/Hot Tub Temp sensor).
  11. Verify that the incorrect outdoor temperature displayed is actually the temperature from the additional temperature and humidity sensor.
  12. The issue occurs consistently every time these steps are performed.

Expected behavior:

The outdoor temperature and the temperatures from extra modules should be stored and displayed separately.

Actual behavior:

The temperature from extra modules is being displayed as the outdoor temperature.

Debug logs:

Here are the debug logs showing the API output with the correct values:

Array
(
    [0] => Array
        (
            [macAddress] => C4:5B:BE:5D:33:FE
            [lastData] => Array
                (
                    // ...
                    [tempf] => 68, // This is the correct outdoor temperature
                    // ...
                    [temp8f] => 100.9, // This is an additional module (Pool/Hot Tub Sensor with temperature only)
                    // ...
                )
            // ...
        )
)

In the above output, [tempf] => 68 is the correct outdoor temperature. However, [temp8f] => 100.9 is actually an additional module which is a Pool/Hot Tub Sensor with temperature only. The issue is that the temperature from this additional module is being displayed as the outdoor temperature.

Suggested fix:

Consider storing the temperature and humidity data from extra modules in separate keys in the $device array to avoid overwriting the outdoor temperature and humidity data. For example:

$device['temperature' . $i] = $device['temp' . $i . 'f'];
$device['humidity' . $i] = $device['humidity' . $i];

This would need to be done in the following section of code:

// NAModule9 - max 9 extra modules
for ($i = 0; $i < 9; $i++) {
    if (array_key_exists('temp' . $i . 'f', $device) || array_key_exists('humidity' . $i, $device)) {
        if (array_key_exists('temp' . $i . 'f', $device)) {
            $device['temperature'] = $device['temp' . $i . 'f'];
        }
        else {
            unset($device['temperature']);
        }
        if (array_key_exists('humidity' . $i, $device)) {
            $device['humidity'] = $device['humidity' . $i];
        }
        else {
            unset($device['humidity']);
        }
        // ...
    }
}

Please note that other parts of the codebase that interact with the $device array might need to be updated to handle these new keys.


Full w/o commentary Debug logs:

Here are the debug logs showing the API output with the correct values:

Array
(
    [0] => Array
        (
            [macAddress] => C4:5B:BE:5D:33:FE
            [lastData] => Array
                (
                    [dateutc] => 1689394440000
                    [tempinf] => 75.6
                    [battin] => 1
                    [humidityin] => 51
                    [baromrelin] => 28.955
                    [baromabsin] => 28.955
                    [tempf] => 68
                    [battout] => 1
                    [battrain] => 1
                    [humidity] => 96
                    [winddir] => 154
                    [winddir_avg10m] => 151
                    [windspeedmph] => 2.2
                    [windspdmph_avg10m] => 5.1
                    [windgustmph] => 6.7
                    [maxdailygust] => 28.4
                    [hourlyrainin] => 0
                    [eventrainin] => 0.24
                    [dailyrainin] => 0.24
                    [weeklyrainin] => 0.874
                    [monthlyrainin] => 3.823
                    [yearlyrainin] => 4
                    [lightning_day] => 144
                    [lightning_time] => 1689382933000
                    [lightning_distance] => 12.43
                    [batt_lightning] => 0
                    [solarradiation] => 0
                    [uv] => 0
                    [temp3f] => 7.7
                    [humidity3] => 73
                    [temp4f] => -4.2
                    [humidity4] => 58
                    [temp5f] => 35.8
                    [humidity5] => 56
                    [temp6f] => 0.1
                    [humidity6] => 54
                    [temp8f] => 100.9
                    [batt3] => 1
                    [batt4] => 1
                    [batt5] => 1
                    [batt6] => 1
                    [batt7] => 1
                    [batt8] => 1
                    [pm25] => 11
                    [pm25_24h] => 23.1
                    [aqi_pm25] => 46
                    [aqi_pm25_24h] => 74
                    [batt_25] => 1
                    [batt_co2] => 1
                    [feelsLike] => 68
                    [dewPoint] => 66.81
                    [feelsLike3] => 7.7
                    [dewPoint3] => 0.9
                    [feelsLike4] => -4.2
                    [dewPoint4] => -15.2
                    [feelsLike5] => 35.8
                    [dewPoint5] => 21.7
                    [feelsLike6] => 0.1
                    [dewPoint6] => -12.6
                    [feelsLikein] => 75.3
                    [dewPointin] => 56.2
                    [lastRain] => 2023-07-15T03:52:00.000Z
                    [lightning_hour] => 0
                    [tz] => America/Chicago
                    [date] => 2023-07-15T04:14:00.000Z
                )

            [info] => Array
                (
                    [name] => Main Farm Weather Station
                    [coords] => Array
                        (
                            [coords] => Array
                                (
                                    [lat] => 41.096889
                                    [lon] => -89.999745
                                )

                            [address] => 2220 Golf Course Rd, La Fayette, IL 61449, USA
                            [location] => La Fayette
                            [elevation] => 228.0929107666
                            [geo] => Array
                                (
                                    [type] => Point
                                    [coordinates] => Array
                                        (
                                            [0] => -89.999745
                                            [1] => 41.096889
                                        )

                                )

                        )

                )

        )

)

jaz-on commented 1 year ago

Hello @dbusch78 thanks for this very detailed issue! <3 Your feedback is noted, as soon as I've got some time to test it, I'll get back to you on this. I'm on holiday until ~September, I'm replying as I had to fix an urgent situation on #23.

Meanwhile, since you seem to have some ideas on how to tackle this issue, would you be willing to submit a PR? I think we could iterate on your proposal quicker. What do you think?