vsergeev / python-periphery

A pure Python 2/3 library for peripheral I/O (GPIO, LED, PWM, SPI, I2C, MMIO, Serial) in Linux.
MIT License
519 stars 139 forks source link

Polling techniques seems a bit inaccurate for sysfs #65

Closed Shubhranshu153 closed 10 months ago

Shubhranshu153 commented 10 months ago
    def poll(self, timeout=None):
        if not isinstance(timeout, (int, float, type(None))):
            raise TypeError("Invalid timeout type, should be integer, float, or None.")

        # Setup poll
        p = select.poll()
        p.register(self._fd, select.POLLPRI | select.POLLERR)

        # Scale timeout to milliseconds
        if isinstance(timeout, (int, float)) and timeout > 0:
            timeout *= 1000

        # Poll
        events = p.poll(timeout)

        # If GPIO edge interrupt occurred
        if events:
            # Rewind
            try:
                os.lseek(self._fd, 0, os.SEEK_SET)
            except OSError as e:
                raise GPIOError(e.errno, "Rewinding GPIO: " + e.strerror)

            return True

        return False

minor -> the timeout should be divided by 1000 not multiplied to convert sec to millisec as select.poll.poll is expecting in second

major: the events are returned as a bitmap that needs to be deciphered probably in a if else kind of state (as we are having or on 2 events). if an error occurs we should return false.

Not sure about this: but i think this part of the code should use https://man7.org/linux/man-pages/man7/inotify.7.html. to detect changes to the files and based on the edge set, it should return true or false. if we can discuss this and come to a solution, will like to code contribute this part.

vsergeev commented 10 months ago

I recommend using character device GPIOs instead, as sysfs GPIOs have been deprecated for some time now. Character device GPIOs also have a richer event API with timestamping.

Regarding the minor point, the polling object returned by select.poll() expects milliseconds for timeout: https://docs.python.org/3/library/select.html#select.poll.poll .

Regarding your major point, the kernel always sets both POLLPRI and POLLERR when polling sysfs (kernfs) files: https://elixir.bootlin.com/linux/v6.5.5/source/fs/kernfs/file.c#L883

As for inotify, in the man page you listed -- "Furthermore, various pseudo-filesystems such as /proc, /sys, and /dev/pts are not monitorable with inotify."