asednev / homebridge-plugin-govee

Govee H-series Thermometer Hygrometer plugin for Homebrige.
Apache License 2.0
53 stars 9 forks source link

Can't report temperatures below freezing on H5075 #16

Closed gawtDamn closed 2 years ago

gawtDamn commented 3 years ago

Describe The Bug: When the sensor of my H5075 goes below 32F (0C), the Home app displays the device's temperature as 212F. In the app "Controller for Homekit" the message traffic for the temperature is missing when the temperature is below freezing, although humidity messages still happen.

To Reproduce: Put a H5075 sensor in the freezer, and watch the message traffic in Controller for Homekit, and in Home.app

Expected behavior: The correct temperature is shown on the sensor display, so I would expect this to be shown in the apps and in the message traffic. I have verified using an Aqara Homekit-compatible temperature sensor that Homekit does support temperatures below freezing. My guess is that the plugin as-written uses a data type that doesn't support negative numbers, and that the native data computations are done in Celsius.

Logs:

Show the Homebridge logs here, remove any sensitive information.

Plugin Config:

Show your Homebridge config.json here, remove any sensitive information.

Screenshots:

Environment: homebridge running on the official rasPi install, 1.1.17

asednev commented 3 years ago

@gawtDamn thank you for this bug report. I will see if I can reproduce the issue by putting my sensor into the freezer. This issue could be due to decoding from binary format.

andrewjbates commented 2 years ago

I added some additional debugging info in my instance to help track this down.

Reading without error:

a4c138e62f98: 88ec000174e66400 {
  uuid: 'a4c138e62f98',
  address: 'a4:c1:38:e6:2f:98',
  model: 'GVH5075_2F98',
  battery: 100,
  humidity: 46.2,
  tempInC: 9.5462,
  tempInF: 49.18316,
  rssi: -84
}

Reading with error (actual temperature was -11.5C and Humidity was 33%)

a4c138fe190a: 88ec0081c2896400 {
  uuid: 'a4c138fe190a',
  address: 'a4:c1:38:fe:19:0a',
  model: 'GVH5075_190A',
  battery: 100,
  humidity: 94.5,
  tempInC: 850.3945,
  tempInF: 1562.7101,
  rssi: -82
}

Here's the relevant code:

export const decodeH5075Values = (streamUpdate: string) => {
    // TODO would be great to find a way to validate

    const encodedData = parseInt(streamUpdate.substring(6, 12), 16);
    const battery = parseInt(streamUpdate.substring(12, 14), 16);
    const tempInC = encodedData / 10000;
    const tempInF = (tempInC * 9) / 5 + 32;
    const humidity = (encodedData % 1000) / 10;

    return {
        battery,
        humidity,
        tempInC,
        tempInF,
    };
};

With that in mind, let's dig into what we're actually getting with each stream update:

scenario streamUpdate encodedData encodedData as Binary
working 88ec000174e66400 0174e6 0000 0001 0111 0100 1110 0110
broken 88ec0081c2896400 81c289 1000 0001 1100 0010 1000 1001

Take note that when the temperature goes negative, there's a leading 8 in the part of the data for temperature which in turn is a leading 1 in the binary data. It's basically a signed 6-byte number? Here's the encodedData fleshed out:

scenario encodedData as Binary Hex2Dec T RH
working 0000 0001 0111 0100 1110 0110 95462 9.5 C 46.2
broken 1000 0001 1100 0010 1000 1001 8503945 850.3 C 94.5
fixed 1000 0001 1100 0010 1000 1001 -115337 -11.5 C 33.7

What's baffling me about this is that the temperature and humidity are stored back to back in decimal, but for the resulting binary number, the first bit is positive or negative based on if the temperature is positive or negative.

I don't have a lot of experience reasoning about bits so I expect there's a better way to handle all this, but I'm imagining the following fix:

1) Get the binary representation of the number 2) If it's got less than 24 digits, it's a positive number 3) If it's got 24 digits, and the first digit is 0, it's positive 4) If it's got 24 digits, and the first digit is 1, it's negative 5) Make a note of positive/negative and then parse min(length,23) digits

This can also be represented as this truth table:

Binary First=0 First=1
Len <24 Positive Positive
Len=24 Positive Negative

~The following is a notion of how to implement it, it's completely freehand and might have syntax errors:~

Here's the rewritten function I'm using in my HomeBridge. It passed a test with the device in the freezer. I edited this in-place in /usr/local/lib/node_modules/homebridge-plugin-govee/node_modules/govee-bt-client/dist for testing. I'm not familiar with how to repackage/test custom HomeBridge plugins so I haven't tested that way yet. I'll eventually get around to testing this properly on my side and will submit a PR unless someone beats me to it

exports.decodeH5075Values = (streamUpdate) => {
    // TODO would be great to find a way to validate

    const encodedData = parseInt(streamUpdate.substring(6, 12), 16);
    const dataAsBits = encodedData.toString(2);

    var tempIsNegative = false;
    var dataToUse = encodedData;

    if (dataAsBits.length == 24) {
      tempIsNegative = (dataAsBits.substr(0,1) == "1");
      dataToUse = parseInt(dataAsBits.substr(1,23), 2);
    }

    const tempInC = dataToUse / 10000 * (tempIsNegative ? -1 : 1);
    const battery = parseInt(streamUpdate.substring(12, 14), 16);
    const tempInF = (tempInC * 9) / 5 + 32;
    const humidity = (dataToUse % 1000) / 10;

    return {
        battery,
        humidity,
        tempInC,
        tempInF,
    };
};