trainman419 / python-cec

Other
172 stars 42 forks source link

is_on() sometimes freezes cec #49

Open mjm987 opened 3 years ago

mjm987 commented 3 years ago

When using is_on() on a Raspberry Pi, the cec driver sometimes freezes completely so a reboot of the Raspi is needed to recover cec functionality. Possibly this crash is caused when my TV does not respond to the request. May be it is a problem in libcec or the raspi kernel driver (?) but I had a quick look in this projects device.cpp file on function _Device_ison(Device * self) and saw that a reference counting is done on a local variable. Isn't this wrong?

I suggest following correction:

--- device.cpp  2020-12-16 17:50:54.178413383 +0100
+++ b.cpp   2020-12-16 17:52:34.042359608 +0100
@@ -85,23 +85,20 @@
    Py_BEGIN_ALLOW_THREADS
    power = adapter->GetDevicePowerStatus(self->addr);
    Py_END_ALLOW_THREADS
-   PyObject * ret;
    switch(power) {
       case CEC_POWER_STATUS_ON:
       case CEC_POWER_STATUS_IN_TRANSITION_ON_TO_STANDBY:
-         ret = Py_True;
+         Py_RETURN_TRUE;
          break;
       case CEC_POWER_STATUS_STANDBY:
       case CEC_POWER_STATUS_IN_TRANSITION_STANDBY_TO_ON:
-         ret = Py_False;
+         Py_RETURN_FALSE;
          break;
       case CEC_POWER_STATUS_UNKNOWN:
       default:
          PyErr_SetString(PyExc_IOError, "Power status not found");
          return NULL;
    }
-   Py_INCREF(ret);
-   return ret;
 }
nforro commented 3 years ago

May be it is a problem in libcec or the raspi kernel driver (?) but I had a quick look in this projects device.cpp file on function _Device_ison(Device * self) and saw that a reference counting is done on a local variable. Isn't this wrong?

No, it's not wrong. Py_INCREF() is called on either Py_True or Py_False, which is the same thing PyRETURN* macros do.

mjm987 commented 3 years ago

ok, thanks for the explanation. Because _ison() does not work reliable on my TV, I added a cec handler in which I check all events recevied for a power standby event. Is there a better solution? Eg. is it possible to set a event filter?