auto-pi-lot / autopilot

Distributed behavioral experiments
https://docs.auto-pi-lot.com
Mozilla Public License 2.0
93 stars 24 forks source link

init_hardware on a Child fails to start pigpiod due to threading problem #186

Open cxrodgers opened 2 years ago

cxrodgers commented 2 years ago

Having trouble initializing hardware on a Child Pi. As far as I can tell, the first time a GPIO is initalized, it creates a subprocess for pigpiod and stores it in a global variable PIGPIO_DAEMON, unless that global already exists, in which case it uses the existing value. Problem occurs because for some reason the signal line can't be executed in any thread except the main one, and I guess the Child task is being run in a thread? I do not get this problem on the Parent Pi.

Seems like the fix is to call external.start_pigpiod() in the main thread on the Child, if I can figure out how to do that.

                    DEBUG    [hardware.gpio.Digital_In.POKES_L] Logger created: hardware.gpio.Digital_In.POKES_L                                                   loggers.py:164
                    ERROR    [tasks.paft_child.PAFT_Child.rpi01] Pin could not be instantiated - Type: POKES, Pin: L                                            paft_child.py:448
                             Got exception:signal only works in main thread of the main interpreter                                                                              
                             ╭────────────────────────────────────────────── Traceback (most recent call last) ───────────────────────────────────────────────╮                  
                             │ /home/pi/dev/autopilot/autopilot/tasks/paft_child.py:427 in init_hardware                                                      │                  
                             │                                                                                                                                │                  
                             │   424 │   │   │   │   │   if isinstance(hw_args, dict):                                                                        │                  
                             │   425 │   │   │   │   │   │   if 'name' not in hw_args.keys():                                                                 │                  
                             │   426 │   │   │   │   │   │   │   hw_args['name'] = "{}_{}".format(type, pin)                                                  │                  
                             │ ❱ 427 │   │   │   │   │   │   hw = handler(**hw_args)                                                                          │                  
                             │   428 │   │   │   │   │   else:                                                                                                │                  
                             │   429 │   │   │   │   │   │   hw_name = "{}_{}".format(type, pin)                                                              │                  
                             │   430 │   │   │   │   │   │   hw = handler(hw_args, name=hw_name)                                                              │                  
                             │                                                                                                                                │                  
                             │ /home/pi/dev/autopilot/autopilot/hardware/gpio.py:763 in __init__                                                              │                  
                             │                                                                                                                                │                  
                             │    760 │   │   """                                                                                                             │                  
                             │    761 │   │                                                                                                                   │                  
                             │    762 │   │   """                                                                                                             │                  
                             │ ❱  763 │   │   super(Digital_In, self).__init__(pin, **kwargs)                                                                 │                  
                             │    764 │   │                                                                                                                   │                  
                             │    765 │   │   # pull the resistor in the off direction and set the trigger to be the on                                       │                  
                             │        direction                                                                                                               │                  
                             │    766 │   │   self.pull = self.off                                                                                            │                  
                             │                                                                                                                                │                  
                             │ /home/pi/dev/autopilot/autopilot/hardware/gpio.py:189 in __init__                                                              │                  
                             │                                                                                                                                │                  
                             │    186 │   │                                                                                                                   │                  
                             │    187 │   │   # init pigpio                                                                                                   │                  
                             │    188 │   │   self.CONNECTED = False                                                                                          │                  
                             │ ❱  189 │   │   self.CONNECTED = self.init_pigpio()                                                                             │                  
                             │    190 │   │                                                                                                                   │                  
                             │    191 │   │   # set default attributes                                                                                        │                  
                             │    192                                                                                                                         │                  
                             │                                                                                                                                │                  
                             │ /home/pi/dev/autopilot/autopilot/hardware/gpio.py:208 in init_pigpio                                                           │                  
                             │                                                                                                                                │                  
                             │    205 │   │   Returns:                                                                                                        │                  
                             │    206 │   │   │   bool: True if connection was successful, False otherwise                                                    │                  
                             │    207 │   │   """                                                                                                             │                  
                             │ ❱  208 │   │   self.pigpiod = external.start_pigpiod()                                                                         │                  
                             │    209 │   │   self.pig = pigpio.pi()                                                                                          │                  
                             │    210 │   │   if self.pig.connected:                                                                                          │                  
                             │    211 │   │   │   return True                                                                                                 │                  
                             │                                                                                                                                │                  
                             │ /home/pi/dev/autopilot/autopilot/external/__init__.py:85 in start_pigpiod                                                      │                  
                             │                                                                                                                                │                  
                             │    82 │   │   │   proc.kill()                                                                                                  │                  
                             │    83 │   │   │   sys.exit(1)                                                                                                  │                  
                             │    84 │   │   atexit.register(kill_proc)                                                                                       │                  
                             │ ❱  85 │   │   signal.signal(signal.SIGTERM, kill_proc)                                                                         │                  
                             │    86 │   │                                                                                                                    │                  
                             │    87 │   │   # sleep to let it boot up                                                                                        │                  
                             │    88 │   │   sleep(1)                                                                                                         │                  
                             │                                                                                                                                │                  
                             │ /usr/lib/python3.9/signal.py:47 in signal                                                                                      │                  
                             │                                                                                                                                │                  
                             │   44                                                                                                                           │                  
                             │   45 @_wraps(_signal.signal)                                                                                                   │                  
                             │   46 def signal(signalnum, handler):                                                                                           │                  
                             │ ❱ 47 │   handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))                                              │                  
                             │   48 │   return _int_to_enum(handler, Handlers)                                                                                │                  
                             │   49                                                                                                                           │                  
                             │   50                                                                                                                           │                  
                             ╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯                  
                             ValueError: signal only works in main thread of the main interpreter  
cxrodgers commented 2 years ago

Ok I think I found the problem.

Workaround: add a pref called PIGPIOD and set it to "true". Fix (TODO): consistently call this variable either PIGPIO or PIGPIOD. Also check whether autopilot.setup is setting it to false (I probably forgot to check this box during setup, but it probably also should default to true if the user forgets)

I would also propose that starting pigpiod should not be a side effect of initializing a hardware pin .. it should only be initialized when the Agent is booted. The current side effect doesn't work anyway because initializing hardware happens in a thread, and threads can't use "signal" (apparently). The only reason "signal" is being used is to connect a "kill" to pigpiod, but this doesn't seem to be working reliably anyway (we have to kill pigpiod on every autopilot restart). So we should probably just remove this use of signal (https://github.com/auto-pi-lot/autopilot/blob/04b5968ba02c8a1413a27eb6a138b6a186b130f1/autopilot/external/__init__.py#L85).

sneakers-the-rat commented 2 years ago

YES. that is extremely annoying. Making Prefs explicit is the thing I didn't have time to get to when switching over to formal data models for v0.5.0. The way it works now is bad for exactly this reason (not clearly documented, some prefs are implicitly accessed, etc.)

related to : https://github.com/auto-pi-lot/autopilot/issues/155

Launching pigpiod as a byproduct of initializing a GPIO object is for the purpose of being able to use them outside of a task/agent context, which was a requested (and useful) feature, but pigpio is just a pretty fussy program generally and so yes we need a better way of killing the process. A better way of launching it than is done now (a function call with an implicit global module state) would be to have explicit process handling classes, just haven't had the time to implement yet.