google-coral / edgetpu

Coral issue tracker (and legacy Edge TPU API source)
https://coral.ai
Apache License 2.0
422 stars 125 forks source link

Dev Board PWM device with python-periphery can get stuck #153

Open FHermisch opened 4 years ago

FHermisch commented 4 years ago

Hi, PWM with python-periphery can get stuck if frequency will be increased: Reproducing the error:

from periphery import PWM
pwm = PWM(0, 0)
pwm.duty_cycle = 0
pwm.frequency = 5000
pwm.duty_cycle = 0.98
pwm.frequency = 5500
OSError: [Errno 22] Invalid argument

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "proverr.py", line 6, in <module>
    pwm.frequency = 5500
  File "/home/mendel/.local/lib/python3.7/site-packages/periphery/pwm.py", line 270, in _set_frequency
    self.period = 1.0 / frequency
  File "/home/mendel/.local/lib/python3.7/site-packages/periphery/pwm.py", line 228, in _set_period
    self.period_ns = int(period * 1e9)
  File "/home/mendel/.local/lib/python3.7/site-packages/periphery/pwm.py", line 179, in _set_period_ns
    self._write_channel_attr("period", str(period_ns))
  File "/home/mendel/.local/lib/python3.7/site-packages/periphery/pwm.py", line 118, in _write_channel_attr
    f_attr.write(value + "\n")
OSError: [Errno 22] Invalid argument

As setting the frequency will change the internal 'period_ns' value but NOT re-calculate the correspoding internal 'duty_cycle_ns' value, it will end up with a 'duty_cycle_ns' value higher then 'period_ns' - which causes errors.

Environment used:
Linux silly-dog 4.14.98-imx #1 SMP PREEMPT Fri Nov 8 23:28:21 UTC 2019 aarch64 GNU/Linux
PRETTY_NAME="Mendel GNU/Linux 4 (Day)"
NAME="Mendel GNU/Linux"
ID=mendel
ID_LIKE=debian
HOME_URL="https://coral.withgoogle.com/"
SUPPORT_URL="https://coral.withgoogle.com/"
BUG_REPORT_URL="https://coral.withgoogle.com/"
VERSION_CODENAME="day"

Proposed workaround:
Always change the duty cycle to 0 before changing the frequency. Otherwise you can end up with an internal 'duty_cycle_ns' value which is higher then 'period_ns' which crashes in the described way. As pwm values are written persistently (in the mendel-devices?), this situation will not fix itself but can only be solved by generating a state with 'duty_cycle_ns < period_ns'. E.g. by setting duty_cycle to 0.

Originally posted by @FHermisch in https://github.com/google-coral/edgetpu/issues/131#issuecomment-647118956

Namburger commented 4 years ago

Hey @FHermisch Thanks for debugging and reporting this. I guess I did't know enough about HW to understand this issue but do you believe that this is a bug?

FHermisch commented 4 years ago

Hi @Namburger Yes, I think it should be treated as a bug. There seems to be some kind of incompatibility between the Mendel linux PWM device and the way this device is accessed and typically used through python-periphery. As the error-state (once reached) will not clean itself on reboot, it may lead people to a time consuming reflash of the board.

I see two possible options to fix this:

  1. Document the problem/ workaround and change board behaviour to reset the PWM state on reboot (to not get people caught in the error state)

  2. Add functionality on the PWM device level to act more graceful once 'duty_cycle_ns > period_ns'

I do not think of this as a bug in python-periphery.

In my opinion, the 2. option will be better, as some existing usages of PWM might rely on the fact that the state stays over a reboot. But I am no low-level Linux guy and cant estimate any effort on this.

There is another issue with PWM which is a hardware or a documentation problem (I think it is a documentation problem but it is quite essential to get PWM running) and we do need some actual hardware people for this. Short: documentation suggests that PWM pins can be used just as GPIOs (but pulsed) and yes they seems to be used that way on RasberrryPis or JetsonNanos. On the dev-board they act like 'open collector' outputs but I havent found a documentation mentioning this. Should I fill another issue on this?

And btw. - by respecting both points, my fan control is now working like a charm ;-)

Namburger commented 4 years ago

@FHermisch Thanks for the details explanation of the issue. I'll raise this up on our team meeting this Wednesday and will decide which action to take regarding this bug!