Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance Enhancements #30

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Performance Enhancements #30

wants to merge 17 commits into from

Conversation

tomjn
Copy link
Contributor

@tomjn tomjn commented Jul 22, 2023

The grow monitor service was using a lot of CPU on my Pi Zero 1 which ate up power, regularly in the 70-90% CPU utilisation.

At this point I thought, does it really have to update, render, and resave the config on every single loop?

So I split it out into 3 functions and use the schedule package, now it's much more likely to be in the 30-60% CPU range, and the main loop is much cleaner.

@tomjn
Copy link
Contributor Author

tomjn commented Jul 22, 2023

One other thing I wanted to try was storing a base image on each view, there's no need to re-render all the icons and backgrounds every single frame, better to render it once, then copy and draw text/charts over the top. I don't know how much this would actually save though

@Gadgetoid
Copy link
Member

This is some great effort toward making this a better script, thank you and sorry it has gone largely ignored. Everything is on fire for me right now, but I'm working through it.

I've upgraded Grow to our latest Python packaging standards, and am now starting to migrate it from RPi.GPIO over to gpiod/gpiodevice. This is quite a significant upheavel and will conflict and make a merry mess of your well intentioned changes.

Would you like to:

  1. Wait for the dust to settle and rebase these changes on top of the updated library

Or:

  1. Let me figure it out (and wrangle github to ensure you're correctly credited as the author)

@tomjn
Copy link
Contributor Author

tomjn commented Nov 17, 2023

Wrangle everything you need to and I'll revisit this, then I can do it in more atomic PRs instead of one great big one.

I actually have one or two bugbears I've been meaning to fix in this, not sure if they affect the main branch too or just my work:

  • my backlight goes off when my fingers shadow crosses the light
  • Pressing the buttons doesn't turn on the backlight
  • sensor 1 doesn't display a bar on the main screen for some reason, but displays fine in the full screen view with the graph, sensor 2 does not have this issue though, very weird

I also determined most of the CPU usage after these changes is from image rendering, and there's newer versions/replacements of the library used.

Something I would suggest though:

  • make the grow config file non-root
  • change the install method so that the monitor isn't installed into the bin folder but instead just a wrapper is installed, that way updating the script is just a git pull away rather than doing vim surgery in the user bin folder as root

@Gadgetoid
Copy link
Member

The monitor script would also be relying on packages in a virtual environment in most cases, so I need to figure out how to run "services" like that in the context of the correct venv 😬

Work in progress for the migration is here - #36

Huge roadblock for this is that gpiod doesn't support soft PWM, nor will it ever. I've tried to implement PWM in pure Python and it works, but it's not stable or reliable. Might have to roll a new, do-one-thing-succinctly PWM library for this.

I haven't tackled edge counting with the sensors yet, either, but in theory gpiod should handle that reasonably well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants