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

[feature] add systemd/dbus service #34

Closed
VuiMuich opened this issue Nov 28, 2021 · 15 comments
Closed

[feature] add systemd/dbus service #34

VuiMuich opened this issue Nov 28, 2021 · 15 comments

Comments

@VuiMuich
Copy link
Contributor

it would be nice, for convenient autostart to have systemd and dbus services 'shipped with' wired, similar as dunst has.

Also documentation on how to setup wired as standard notification daemon would be nice.

@Toqozz
Copy link
Owner

Toqozz commented Nov 29, 2021

Personally I prefer just putting it in an autostart file, since I find services a bit opaque/unnecessary for simple stuff.

Having said that, I'll concede there are some advantages to having a service, like auto-start on notification send and resilience to crashing, though. Maybe having both is an option.

What do you mean by setting up wired as the standard notification daemon?

@VuiMuich
Copy link
Contributor Author

At first I had dunst still installed parallel and I believe due to the crashes of #22 systemd kept reawakening dunst.
If both daemons have a service it might be a bit easier to set the "priority".

I don't have much experience creating new services, tbh, but I can try and open a pr eventually, if you like.

@Toqozz
Copy link
Owner

Toqozz commented Nov 29, 2021

Ah, I see what you mean. Yeah, we can/should do something documenting that.

A PR would be welcome, but I will also get around to this eventually.

@Luviz
Copy link

Luviz commented Dec 6, 2021

Not a systemd expert but this works for me.

[Unit]
Description=wired-notify deamon
After=sddm.service

[Install]
WantedBy=multi-user.target

[Service]
ExecStart=/bin/bash -c /bin/wired -r
RestartSec=10
Restart=always

put this file in /usr/lib/systemd/user/wired_notify.service and then use can enable it using --user flag

enable:

sudo systemctl --user enable wired_notify.service

I wasn't able to get it auto starting but I put this in my session start up script.

systemctl --user start wired_notify.service &

I would love to see this in the main branch but I can't atm.

@MatanHamilis
Copy link
Contributor

MatanHamilis commented Jan 1, 2022

I have various points that should be taken into consideration when adding a systemd service which I'll share in this comment.

  1. Xorg/wayland, do we even support both? It may affect the solution we choose eventually.
  2. Display managers: Let's consider Luviz's snippet:
[Unit]
Description=wired-notify deamon
After=sddm.service

[Install]
WantedBy=multi-user.target

[Service]
ExecStart=/bin/bash -c /bin/wired -r
RestartSec=10
Restart=always

This is a good attempt and it looks good overall, but there's one issue with it.
This service assumes the existence of a particular display manager (i.e. sddm) which is typical for KDE/Plasma users, for example.
Many others who use GNOME for instance, use other display manager (name gdm3 IIRC).
Similarly, we have xdm, lightdm any many others...
Personally, I don't use any display manager but only a window manager, so I think we shouldn't rely on the existence of any display manager.

  1. Systemd's tendency to silently restart crashing services may prevent us from getting to know about bugs, is this a thing we want?
  2. To prevent the installer script from requiring root permissions, we should consider placing the systemd service at ~/.config/systemd/user/ (depending on XDG environment). Since users typically backup their ~/.config directory, they will carry this service file with them, which is nice :)

If we want to rely on Xorg, then we probably want to started wired when Xorg is beginning, I don't know how to describe a dependency on Xorg in systemd-service since Xorg may be started not using a systemd-service.

@Toqozz
Copy link
Owner

Toqozz commented Jan 1, 2022

Nice input, thanks.

Xorg/wayland, do we even support both? It may affect the solution we choose eventually.

Wired is completely untested on Wayland and I don't even know what it would take to support it. As far as I understand everything is tied much more closely to the compositor so it wouldn't even be possible to support the same way we do on X11 (?). mako just says "works on sway", whatever that means.

Systemd's tendency to silently restart crashing services may prevent us from getting to know about bugs, is this a thing we want?

Great point and one of the reasons I haven't pushed for a systemd service so far. The other reason is that I don't really get the point of such a complex start system for a program as simple as Wired. Why over-complicate wired &? One reason is auto-restarting after crashing, but see thoughts above.

@Luviz
Copy link

Luviz commented Jan 1, 2022

Awesome, and thanks for support.
there is a one thing in this dissociation that I want to point out.

  1. Systemd's tendency to silently restart crashing services may prevent us from getting to know about bugs, is this a thing we want?

The hole reson why I wrote the unit file was because, wired was crashing on me, and bluemans notification don't auto close.
So in short I want the auto restart, as you can see in bottom two lines of the snippet. note; that if you remove Restart=always systemd will not restart the program.

I can't speak for anyone else but I don't have 8~16h to spend on debugging a foreign code base. Which is why I am so thankful to the open source community and I try to add to as much as I can!

So yes! This is a dock-tape solution, but it took me < 2 hours to put together.

As for not being able to detected bugs, Systemd devs already thought about it. The solutions is log files which you can access via journalctl.

$ journalctl --user /usr/bin/wired

image

image

Again, thanks for an awesome software!

@MatanHamilis
Copy link
Contributor

The hole reson why I wrote the unit file was because, wired was crashing on me, and bluemans notification don't auto close. So in short I want the auto restart, as you can see in bottom two lines of the snippet. note; that if you remove Restart=always systemd will not restart the program.

I can't speak for anyone else but I don't have 8~16h to spend on debugging a foreign code base. Which is why I am so thankful to the open source community and I try to add to as much as I can!

It looks like we were facing the same issue, this is a good thing, my wired instance has also been crashing.

So yes! This is a dock-tape solution, but it took me < 2 hours to put together.
I thought of using this solution as a basis for a broader and stabler solution we can push upstream.

As for not being able to detected bugs, Systemd devs already thought about it. The solutions is log files which you can access via journalctl.

I've been using journalctl and never thought of using it that way, sweet! Though I still think that users won't bother to report these log messages if they aren't aware of the bug.


In summary, I see two independent issues here.

  1. Lack of a service that will recover wired in case of a crash.
  2. The original issue @Luviz is experiencing which leads to his instance crashing (which I guess should be reported in a separate issue).

In an attempt to keep focusing on the systemd aspect, is there any way we can trigger wired to start when Xorg is starting?

@Toqozz
Copy link
Owner

Toqozz commented Jan 1, 2022

  • Crashing is a bad user experience.
  • We can bandaid the issue by having a service which restarts the program.
  • This might hide the issue from some people, and the issue may be fixed less quickly.

Obviously, as the maintainer of Wired I would much prefer to catch bugs ASAP, but I understand that the bandaid approach does have real benefits -- no software is perfect. I'm not totally opposed to providing a service if we get to something workable here.


By the way @Luviz, from the screenshot you posted you might be experiencing #22 , which is fixed in master, and will be released soon. @MatanHamilis if your crash is separate, if you could just post the output in a separate issue, ideally a debug build.

@MatanHamilis
Copy link
Contributor

MatanHamilis commented Jan 1, 2022 via email

@Toqozz
Copy link
Owner

Toqozz commented Jan 1, 2022

My issue is exactly the same, already tried v0.9.5 too see the crashes are fixed.

0.9.5 doesn't exist yet. Do you mean master?

@MatanHamilis
Copy link
Contributor

MatanHamilis commented Jan 1, 2022 via email

@VuiMuich
Copy link
Contributor Author

VuiMuich commented Jan 2, 2022

I have written this unit file with inspiration of the one from dunst

[Unit]
Description=Wired Notification Daemon
PartOf=graphical-session.target

[Service]
Type=dbus
BusName=org.freedesktop.Notifications
ExecStart=/usr/bin/wired

[Install]
WantedBy=graphical-session.target

I placed it at /usr/lib/systemd/user/wired.service and activated it with systemd --enable --now --user wired.service
I have yet to test, if it starts correctly after a system reboot, as my previous attempt did not.

I think it would be nice to just have such a unit file in the repo, so package maintainers would have a unified source for systemd support.
In addition a small section in the README and/or the wiki about systemd support would be helpful for those building from source.

@Toqozz
Copy link
Owner

Toqozz commented Jan 3, 2022

Thanks for the service!

I've added it to the root of the repository and added a small section on how to use it in the README for now.

@VuiMuich
Copy link
Contributor Author

VuiMuich commented Jan 3, 2022

Also thanks for adding!

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

No branches or pull requests

4 participants