sfalkman / ngenic-hass-platform

Ngenic platform for Home Assistant
MIT License
33 stars 17 forks source link

Get zero values now and then #32

Open rindlerblabla opened 3 years ago

rindlerblabla commented 3 years ago

First, thanks for the integration! Don't really know how to check what is wrong, but now and then I get zero values from the outdoor thermometer. No error codes in the log.

Screenshot_20210530-213429_Home Assistant

sfalkman commented 3 years ago

Hmm, that's strange.. Does it report a 0-value or does it say that it was unavailable during that time? You could enable debug-level logs for the component to get a bit more info on what's happening.

rindlerblabla commented 3 years ago

It reports a 0-value. Think I have enabled logging in the correct way now. Will get back to you.

rindlerblabla commented 3 years ago

Now it happened again, however it seems I didn't setup the debug config correct? Don't get anything in the regular log at least.

logger: default: warning logs: custom_components.ngenic: debug

Screenshot_20210605-151337_Home Assistant Screenshot_20210605-150240_Home Assistant

sfalkman commented 3 years ago

Hmm, the below is the config that I use, and that works. Do you see any debug logs from ngenic when starting homeassistant?

logger:
  default: info
  logs:
    custom_components.ngenic: debug

It looks like the component reports "0" for quite some time? Can you see the same thing in the ngenic app while this is happening in homeassistant?

rindlerblabla commented 3 years ago

Obviously the error is on the Ngenic side. In the app there are values missing as well. However in the HA integration those missing data points end up as zero values, while when I have requested values from the API (with Homey) the values just seems to not be updated.

Screenshot_20210605-220247_Ngenic Tune

From Homey (http requests from the API)

Screenshot_20210605-215813_Chrome

Now I have found out where I can see the debug logs, but my HA setup is restarted so the log from earlier today is missing, hehe. Can keep this issue open until I have caught the log.

rindlerblabla commented 3 years ago

2021-06-07 23:24:11 INFO (MainThread) [custom_components.ngenic.sensor] Measurement not found for period, this is expected when data have not been gathered for the period (type=MeasurementType.TEMPERATURE, from=None, to=None)

2021-06-07 23:24:11 DEBUG (MainThread) [custom_components.ngenic.sensor] New measurement: 0.000000 (name=Ngenic controller, type=MeasurementType.TEMPERATURE

sfalkman commented 3 years ago

Sorry for the late reply.
I've debugged this a bit myself, and found that if I pull the batteries from my control box I'll have the same outcome as you do: a 0-value.

You are most likely losing the connection to you control box, moving it closer to your hub will likely solve the issue.

Code-wise, it seems that when the control box lose the connection, the API will still report the control box as available but no measurements are reported. In the hass-component, we interpret this as value 0 because other sensors that fetch measurements between an interval are expected to get no measurements back (hence the "this is expected when data..." message).

Of course, this could probably be improved by separating regular sensor-fetches from interval fetches, and interpret regular fetches that doesn't return any measurements as "Unavailable", but I'm not sure you'll be more happy about that result.

rindlerblabla commented 3 years ago

Thanks for your debugging! In short I think it would be better if the sensor would be reported as Unavailable instead of showing 0 value when there are not new measurements.

Right now there would be no difference for me in HA, but I think the solution is better.

I have contacted Ngenic for their support. The signal strength seems to be just fine, but it might be caused by low battery(?). In the app view it still reports about 30-40% but it is at least worth a try changing them.

sfalkman commented 3 years ago

Yes I agree with you, but I still want to maintain the old functionality for when this is used by a periodic sensor (reporting 0) because I'm unsure about the impact if I were to change that.

I will update to print different messages depending on if a period is set or not.
I will do this when I get some spare time over, got a lot of other stuff going on right now :)
Storing my suggestion here:

    if not measurement:
        # measurement API will return None if no measurements were found
        if kwargs.get("from_dt) is None and kwargs.get("to_dt) is None:
            # if a period was not set, this indicates an error (i.e. the sensor is unavailable)
            _LOGGER.info("Measurement not found, this may be because the sensor is unavailable (type=%s)" %
                (
                    kwargs.get("measurement_type", "unknown")
                )
            )
        else:
            # if a period is set, this might mean no measurements were found for the period
            _LOGGER.info("Measurement not found for period, this is expected when data have not been gathered for the period (type=%s, from=%s, to=%s)" % 
                (
                    kwargs.get("measurement_type", "unknown"), 
                    kwargs.get("from_dt", "None"), 
                    kwargs.get("to_dt", "None")
                )
            )
        # Return None; it's up to the caller to figure out what to do if no measurements are available
        measurement_val = None
class NgenicSensor(Entity):
    ...
    async def _async_fetch_measurement(self):
        ...
        current = await get_measurement_value(self._node, measurement_type=self._measurement_type)
        if current is None:
            # This will log the exception an make the sensor unavailable
            raise Exception("No measurements were returned")
        else:
            return round(current, 1)

class NgenicEnergySensor(NgenicSensor):
    ...
    async def _async_fetch_measurement(self):
        ...
        current = await get_measurement_value(self._node, measurement_type=self._measurement_type, from_dt=from_dt, to_dt=to_dt, period="P1D")
        if current is None:
            current = 0
        return round(current, 1)
rindlerblabla commented 3 years ago

Thanks, and no hurry! Feel free to close this issue whenever you want then.

johanhartin commented 1 year ago

Yes! Please make the sensor report unavailable instead of showing 0.

This would make it possible to discover and fix the issue. I tried to automate a restart of my router when nGenic loses connection but it's very random when temperature is actually zero.