xorbit / LiFePO4wered-Pi

Access library, command line tool and daemon for the LiFePO4wered/Pi module
GNU General Public License v2.0
132 stars 31 forks source link

Add py3, systemd, and packaging support #28

Closed smurfix closed 5 years ago

smurfix commented 5 years ago

This bunch of patches adds

xorbit commented 5 years ago

Hi Matthias,

This is awesome, thank you for your contribution! I can tell you know much more about system integration than I do, so this is very welcome indeed. Before I merge it, I've looked into the code but I'd like a little more information from you to make sure I understand what all changes:

smurfix commented 5 years ago

Yes it might make sense to use a "real" Makefile. I'll do a separate pull request for that.

I did find a problem with the changes, have pushed an update.

Yes it works without systemd, the daemon will behave as before (i.e. fork itself into the background etc).

The systemd watchdog feature requests a message from the program in question, every however-many-seconds-are-configured (in "systemdscript"). If that message doesn't arrive in time, the daemon will get killed and restarted. It is a generic feature and not related to the watchdog in the LiFePO4wered/Pi+.

Systemd has its own system-wide watchdot that could conceivably be expanded to talk to the LiFePO4wered/Pi+, but that's a separate issue which I'll have to talk about with the systemd people, as their watchdog support uses the kernel's watchdog code.

xorbit commented 5 years ago

Understood. May I request one more thing? Could you update the README.md to reflect the changes and new options you added? Not only will this be helpful for my customers, but since I don't quite understand what was changed, I am in no position to do this. You as the author of the update know best what should be documented about the changes.

smurfix commented 5 years ago

Pushed. I'll rewrite it further when I rewrite the build system to use a Makefile.

xorbit commented 5 years ago

I just merged this into the py3-systemd-pkg branch of my repo and tried it on Raspbian Buster on a Pi 4. Yeah, I know, I'm throwing everything at it at once. There seems to be an issue with the daemon. I'm going to try next whether my original code works on Buster, but I wanted to capture this before I switched to my master branch for testing. sudo service lifepo4wered-daemon status returned this:

● lifepo4wered-daemon.service - Daemon for lifepo4wered Pi hat
   Loaded: loaded (/etc/systemd/system/lifepo4wered-daemon.service; disabled; vendor preset: enabled)
   Active: activating (auto-restart) (Result: watchdog) since Fri 2019-06-28 19:20:53 MDT; 4s ago
  Process: 962 ExecStart=/usr/local/sbin/lifepo4wered-daemon (code=killed, signal=ABRT)
 Main PID: 962 (code=killed, signal=ABRT)
   Status: "Startup"

Since I know next to nothing about systemd, I thought I'd pass it on to you.

smurfix commented 5 years ago

I'll investigate shortly. As usual, not enough time …

NB, the most likely cause for this is that you didn't install libsystemd-dev before building the lifepo4wered daemon.

ristomatti commented 5 years ago

@xorbit As a watcher of this issue - based on what @smurfix said, I think the dependency should be added to the README.md and then it should work.

xorbit commented 5 years ago

Hmm, I did follow the exact instructions in the README.md, which in the updated version says to use this command:

sudo apt-get -y install build-essential git python libsystemd-dev

So I don't think that's it.

ristomatti commented 5 years ago

Ah sorry! I had missed the commit updating the README.md above.

xogium commented 4 years ago

this cannot work properly for one simple good reason: @smurfix is enabling watchdog in his code, but does not do anything else. Wakes up the watchdog, and then do nothing results in systemd killing the process because it did not ping the watchdog. You need code for this too, not just a setting in an unit file. Your code needs to handle pings, and your service file should handle the ping interval (1 second is way, way, way too often).

xorbit commented 4 years ago

Ok, thanks for the explanation. I don't know the first thing about systemd, so I was glad someone contributed this. Only like you said, it doesn't work and these has been no follow-up. I would love for someone to make this work.