Migration to gpiozero - Githubissues

ch3p4ll3 commented 2 months ago

Migration to the gpiozero library to make it compatible to the Raspberry Pi5 as well. This will resolve the problems from issue#255.

RobinTail commented 2 months ago

There is an issue in CI, @ch3p4ll3 with installing lgpio:

Building wheels for collected packages: lgpio
  Building wheel for lgpio (setup.py): started
  Building wheel for lgpio (setup.py): finished with status 'error'
  error: subprocess-exited-with-error

  × python setup.py bdist_wheel did not run successfully.
  │ exit code: 1
  Running setup.py clean for lgpio
  ╰─> [15 lines of output]
      running bdist_wheel
      running build
      running build_py
      running build_ext
      building '_lgpio' extension
      swigging lgpio.i to lgpio_wrap.c
      swig -python -o lgpio_wrap.c lgpio.i
      creating build
      creating build/temp.linux-x86_64-3.8
      gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -Isrc -I/opt/hostedtoolcache/Python/3.8.18/x64/include/python3.8 -c lgpio_wrap.c -o build/temp.linux-x86_64-3.8/lgpio_wrap.o
      creating build/lib.linux-x86_64-3.8
      gcc -shared -Wl,--rpath=/opt/hostedtoolcache/Python/3.8.18/x64/lib -Wl,--rpath=/opt/hostedtoolcache/Python/3.8.18/x64/lib build/temp.linux-x86_64-3.8/lgpio_wrap.o -L/opt/hostedtoolcache/Python/3.8.18/x64/lib -llgpio -o build/lib.linux-x86_64-3.8/_lgpio.cpython-38-x86_64-linux-gnu.so
      /usr/bin/ld: cannot find -llgpio: No such file or directory
      collect2: error: ld returned 1 exit status
      error: command 'gcc' failed with exit status 1
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for lgpio
Failed to build lgpio
ERROR: ERROR: Failed to build installable wheels for some pyproject.toml based projects (lgpio)

Could you take a look on what it takes to fix it?

That seems to be happening only in Python 3.7 and 3.8 — perhaps those versions are not supported?

ch3p4ll3 commented 2 months ago

There is an issue in CI, @ch3p4ll3 with installing lgpio:

Building wheels for collected packages: lgpio
  Building wheel for lgpio (setup.py): started
  Building wheel for lgpio (setup.py): finished with status 'error'
  error: subprocess-exited-with-error

  × python setup.py bdist_wheel did not run successfully.
  │ exit code: 1
  Running setup.py clean for lgpio
  ╰─> [15 lines of output]
      running bdist_wheel
      running build
      running build_py
      running build_ext
      building '_lgpio' extension
      swigging lgpio.i to lgpio_wrap.c
      swig -python -o lgpio_wrap.c lgpio.i
      creating build
      creating build/temp.linux-x86_64-3.8
      gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -Isrc -I/opt/hostedtoolcache/Python/3.8.18/x64/include/python3.8 -c lgpio_wrap.c -o build/temp.linux-x86_64-3.8/lgpio_wrap.o
      creating build/lib.linux-x86_64-3.8
      gcc -shared -Wl,--rpath=/opt/hostedtoolcache/Python/3.8.18/x64/lib -Wl,--rpath=/opt/hostedtoolcache/Python/3.8.18/x64/lib build/temp.linux-x86_64-3.8/lgpio_wrap.o -L/opt/hostedtoolcache/Python/3.8.18/x64/lib -llgpio -o build/lib.linux-x86_64-3.8/_lgpio.cpython-38-x86_64-linux-gnu.so
      /usr/bin/ld: cannot find -llgpio: No such file or directory
      collect2: error: ld returned 1 exit status
      error: command 'gcc' failed with exit status 1
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for lgpio
Failed to build lgpio
ERROR: ERROR: Failed to build installable wheels for some pyproject.toml based projects (lgpio)

Could you take a look on what it takes to fix it?

That seems to be happening only in Python 3.7 and 3.8 — perhaps those versions are not supported?

Mmm interesting... Ok, I'll take a look

ch3p4ll3 commented 2 months ago

There is an issue in CI, @ch3p4ll3 with installing lgpio:

Building wheels for collected packages: lgpio
  Building wheel for lgpio (setup.py): started
  Building wheel for lgpio (setup.py): finished with status 'error'
  error: subprocess-exited-with-error

  × python setup.py bdist_wheel did not run successfully.
  │ exit code: 1
  Running setup.py clean for lgpio
  ╰─> [15 lines of output]
      running bdist_wheel
      running build
      running build_py
      running build_ext
      building '_lgpio' extension
      swigging lgpio.i to lgpio_wrap.c
      swig -python -o lgpio_wrap.c lgpio.i
      creating build
      creating build/temp.linux-x86_64-3.8
      gcc -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -Isrc -I/opt/hostedtoolcache/Python/3.8.18/x64/include/python3.8 -c lgpio_wrap.c -o build/temp.linux-x86_64-3.8/lgpio_wrap.o
      creating build/lib.linux-x86_64-3.8
      gcc -shared -Wl,--rpath=/opt/hostedtoolcache/Python/3.8.18/x64/lib -Wl,--rpath=/opt/hostedtoolcache/Python/3.8.18/x64/lib build/temp.linux-x86_64-3.8/lgpio_wrap.o -L/opt/hostedtoolcache/Python/3.8.18/x64/lib -llgpio -o build/lib.linux-x86_64-3.8/_lgpio.cpython-38-x86_64-linux-gnu.so
      /usr/bin/ld: cannot find -llgpio: No such file or directory
      collect2: error: ld returned 1 exit status
      error: command 'gcc' failed with exit status 1
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for lgpio
Failed to build lgpio
ERROR: ERROR: Failed to build installable wheels for some pyproject.toml based projects (lgpio)

Could you take a look on what it takes to fix it?

That seems to be happening only in Python 3.7 and 3.8 — perhaps those versions are not supported?

Ok, so I found out that unfortunately lgpio supports python>=3.9 [1] :(

RobinTail commented 2 months ago

Ok, so I found out that unfortunately lgpio supports python>=3.9 [1] :(

Ok. Should we utilize another pin factory or should we drop Python 3.7 and 3.8 in favour of lgpio? What do you think, @ch3p4ll3 ?

In case you believe lgpio is a must, you can adjust this block in the CI workflow: https://github.com/borisbu/OctoRelay/blob/7c15a7077766a7256b77603cfc00b91284b57ca6/.github/workflows/CI.yaml#L54-L83

and this constant in the main file: https://github.com/borisbu/OctoRelay/blob/7c15a7077766a7256b77603cfc00b91284b57ca6/octoprint_octorelay/__init__.py#L374

An example here: https://github.com/borisbu/OctoRelay/pull/215 — I'm actually looking forward to drop at least 3.7.

ch3p4ll3 commented 2 months ago

Ok, so I found out that unfortunately lgpio supports python>=3.9 [1] :(

Ok. Should we utilize another pin factory or should we drop Python 3.7 and 3.8 in favour of lgpio? What do you think, @ch3p4ll3 ?

In case you believe lgpio is a must, you can adjust this block in the CI workflow:

https://github.com/borisbu/OctoRelay/blob/7c15a7077766a7256b77603cfc00b91284b57ca6/.github/workflows/CI.yaml#L54-L83

and this constant in the main file:

https://github.com/borisbu/OctoRelay/blob/7c15a7077766a7256b77603cfc00b91284b57ca6/octoprint_octorelay/__init__.py#L374

An example here: #215 — I'm actually looking forward to drop at least 3.7.

From the tests I have done on the Pi5, lgpio is the only pin factory that works, other pin factories are not compatible with the new GPIO hardware. This would make lgpio the only option available for raspberry Pi 5 if we want to use this approach.

As far as dropping python 3.7/8, I don't know, the problem is that right now octoprint still supports both python 3.7 and 3.8. If we have to drop support, for some it might be necessary to upgrade python to at least 3.9. Imho this decision should not be up to me but from the repo owners

RobinTail commented 2 months ago

lgpio is the only pin factory that works lgpio the only option available for raspberry Pi 5

Thank you for making that research! Highly appreciate it, @ch3p4ll3

octoprint still supports both python 3.7 and 3.8 this decision should not be up to me but from the repo owners

I'm OK with it. Python 3.7 is EOL, 3.8 is about to Plugins may have more narrow system requirements than the primary application in order to run their features — this is fine. We can release a new major version with a breaking change to Python compatibility in order to offer the Raspberry 5 support.

Please continue :)

RobinTail commented 2 months ago

The installation was successful, @ch3p4ll3 . Now unit tests need to be updated. You can run them locally using the following command from the tests directory:

python -m unittest test*.py

Or just follow the CI steps. You'd need to mock gpiozero instead of RPi.GPIO in tests. Or find the CI execution log here: https://github.com/borisbu/OctoRelay/actions/runs/9820228078/job/27115376938?pr=274

Let me know if you need help.

ch3p4ll3 commented 2 months ago

The installation was successful, @ch3p4ll3 . Now unit tests need to be updated. You can run them locally using the following command from the tests directory:
python -m unittest test*.py
Or just follow the CI steps. You'd need to mock gpiozero instead of RPi.GPIO in tests. Or find the CI execution log here: https://github.com/borisbu/OctoRelay/actions/runs/9820228078/job/27115376938?pr=274

Let me know if you need help.

Hi, I'm going to try to update them tomorrow, maybe I'll need some help since I've never used Mock and from what I've seen they don't look very simple to me lmao

ch3p4ll3 commented 2 months ago

Hi @RobinTail , I tried to update test_driver, could u please take a look, idk if that's good enought tbh. If that's ok I'll start working also on test_init

RobinTail commented 2 months ago

@ch3p4ll3 , well, it's kinda OK, but:

could you please avoid changing the test data (pins) in the tests
the test for Relay::constructor should test only the constructor. The new static method you made should have a new dedicated test (ensure that it calls the constructor OR uses the cache depending on circumstances)

I left a couple notes. But in general, that could work. In the test of main, the whole driver is mocked, so it should be plausible. Thank you

RobinTail commented 2 months ago

Something fails in running the tests — I think it can be resolved by moving test_get() into the existing test_driver.py suit. You can make it a single test for the static method as advised above, @ch3p4ll3 You're doing great 👍🏽

ch3p4ll3 commented 2 months ago

Hi @RobinTail I also fixed test_init.py by adding a Mock for get_or_create_relay and updating py version

RobinTail commented 2 months ago

@ch3p4ll3 the unit tests passed — awesome!

A couple refinements needed to fix the found code style issues:

octoprint_octorelay/driver.py:3: error: Skipping analyzing "gpiozero.pins.lgpio": module is installed, but missing library stubs or py.typed marker  [import-untyped]
octoprint_octorelay/driver.py:3: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
octoprint_octorelay/driver.py:4: error: Skipping analyzing "gpiozero": module is installed, but missing library stubs or py.typed marker  [import-untyped]
octoprint_octorelay/driver.py:8: error: Need type annotation for "relays" (hint: "relays: list[<type>] = ...")  [var-annotated]
Found 3 errors in 1 file (checked 8 source files)

Regarding the first one

In the file "mypy.ini" find the following block:

[mypy-RPi.*]
ignore_missing_imports = True

and replace "RPi" with "gpiozero"

For the second

Add : list[LED] next to the relays in Relay class.

That should fix it.

RobinTail commented 2 months ago

Awesome. Last minor issues, @ch3p4ll3 :

************* Module octoprint_octorelay.driver
octoprint_octorelay/driver.py:45:0: C0303: Trailing whitespace (trailing-whitespace)
************* Module test_driver
tests/test_driver.py:70:0: C0303: Trailing whitespace (trailing-whitespace)
tests/test_driver.py:77:0: C0303: Trailing whitespace (trailing-whitespace)
tests/test_driver.py:4:0: W0611: Unused Mock imported from unittest.mock (unused-import)

coveralls commented 2 months ago

coverage: 99.831% (-0.2%) from 100.0% when pulling dec1718409d1c6211ba7ccd468220ded3713afb3 on ch3p4ll3:master into 7f8694a15f0bc80a6095cfbe7ea68e8010306891 on borisbu:develop.

RobinTail commented 2 months ago

Brilliant, @ch3p4ll3 , thank you! I'm going to merge it to develop and publish a dev-preview so that you could test in on Raspberry Pi 5 and me on Raspberry Pi 4.

RobinTail commented 1 month ago

@ch3p4ll3 , see: https://github.com/borisbu/OctoRelay/pull/280#issuecomment-2218107437

borisbu / OctoRelay

Migration to gpiozero #274

Regarding the first one

For the second