crownstone / bluenet

Bluenet is the in-house firmware on Crownstone hardware. Functions: switching, dimming, energy monitoring, presence detection, indoor localization, switchcraft.
https://crownstone.rocks
91 stars 62 forks source link

Bug: AccumulatedEnergy in PowerSampling fails to increment for high values due to float addition #186

Closed martijnvandermarel closed 2 years ago

martijnvandermarel commented 2 years ago

Describe the bug For some crownstones, the accumulatedEnergy advertised in serviceData remains constant, even though the advertised realPowerUsage is high enough that the accumulatedEnergy should increase over time.

For example, I have a crownstone that consumes ~27 Watt as per the consumer app (see screenshot). Since the accumulatedEnergy field in the advertisement data contains energy in 64J units, the accumulatedEnergy value should increment every ~2 seconds or so. Instead, it remains constant for at least hours (see second screenshot).

To Reproduce onlinegdb simulation for reproducing the bug and the proposed solution. The bug is not easily reproduced on a crownstone since it only occurs for specific (high enough) initial values of the _energyUsedmicroJoule variable in cs_PowerSampling.cpp. In my case it was 17694528000000 (it is an int64_t).

Cause and solution Together with @vliedel I found the cause of the bug to stem from a bug in the incrementation of the energyUsedmicroJoule on line 1036 of cs_PowerSampling.cpp. This operation adds a float to an int64_t using float addition. For high values of the int64_t variable, it seems that the float addition fails (probably due to bad resolution for floats for high numbers). The solution is simple: cast to int64_t before performing the addition. @vliedel and I tested this in the onlinegdb simulation and it seems to work.

Screenshots image image

Binary (please complete the following information):

vliedel commented 2 years ago

Very interesting bug indeed :) Fixed with https://github.com/crownstone/bluenet/commit/6c1ec3d23e2418bf61e061ed57ce3715894896cc