andrew-bibb / cmst

QT GUI for Connman
174 stars 37 forks source link

notify-send for OSD notfications #13

Closed ghost closed 10 years ago

ghost commented 10 years ago

Hi Andrew,

Out of the first request I made, this is one of the issues I mentioned that I'm separating, that you asked for.

cmst needs support for notify-send for OSD notfications.

As I mentioned before I use xfce4-notifyd, but this is just a front end to notify-send.

I started wicd-gtk so you can see how it looks with xfce4-notifyd, again not something you need to be concerned with from what I understand, you just need to code cmst to work with notify-send.

http://i.imgur.com/UVk1gqt.png http://i.imgur.com/gTAYjzB.png http://i.imgur.com/0nhhWqe.png

wicd-gtk uses the same words for the notify-send for wifi, except of course at the top it shows the wifi signal icon and the SSID name it's connecting to...

I made the SSID cmst-test for you to see this is all it uses the SSID;

http://i.imgur.com/EMqjy3x.png http://i.imgur.com/iTDFg9H.png http://i.imgur.com/hlQymko.png

Thank you...

ghost commented 10 years ago

On the new notifications in cmst, when first enabling the preference, and then clicking the wired on and off, it kills the dbus and throws up some errors like these below.

There were some more error messages if I kept clicking OK, but hopefully you get the idea with these two...

http://i.imgur.com/qlLsZrO.jpg http://i.imgur.com/RIwyfY8.jpg

andrew-bibb commented 10 years ago

I think it was a race, I've removed the accelerator from toggle power, but it is hard to know if I got it as it does not happen all the time. Does the version in GIT now appear to solve this at your end? The toggle power will not be quite as responsive as it was, but if this is the problem I may be able to improve that.

ghost commented 10 years ago

Nope seem the same, here's the steps I did...

  1. Logged out of i3 and upgraded cmst, then logged back in and noticed cmst sitting in the tray.
  2. The Wired showed green and I clicked it then got this message, 'There was an error reading or parsing the reply from method connman.Manager.GetProperties.'
  3. I clicked the OK button and recieved the same type of message, so I'll show the 'from section'; connman.Manager.GetTechnologies
  4. I clicked the OK button again and got this one next; connman.Manager.GetServices
  5. I clicked the OK button again and got this one next; connman.Manager.GetProperties (Popup appears saying connman offline)
  6. I clicked the OK button again and got this one next; connman.Manager.GetTechnologies
  7. I clicked the OK button again and got this one next; connman.Manager.GetServices (Popup appears saying connman ready)

Now the popups are gone and Wired shows green, I then try to click it and cmst crashes and dissapears from they tray.

Then I have to restart connmand to get cmst to show up in the tray and working again...

Once I get connmand restarted and I restart cmst, before I also had Retain Settings checked and Enable System Tray, but they were unchecked when I restarted cmst.

andrew-bibb commented 10 years ago

How exactly are you starting cmst with i3? Based on your step 1, was cmst running in the background while you did the upgrade? If you are starting inside i3 it would be helpful to see the exact startup line you are using.

I'm wondering if maybe I need to force a dbus disconnect when the program closes.

edit: researched the force disconnect and I now don't think that is necessary (or even possible). I can reproduce the crash in 14.06.07-1 (at least that one, just did it, and maybe others of that vintage), but try as I might I can't get 14.06.15-3 to crash.

The error messages are exactly correct and in the correct order, but I'm not sure why there are 2 copies. That is probably a clue to something.

edit 2: scratch that, I just managed to force 14.06.15-3 to crash.

edit 3: Just uploaded 14.06.16-1, if you have the time can you try it? I could reproduce the crash in edit 2 at will, but not with the newest version. I'm hopeful this is it this time, but if not let me know what you are getting. Thanks...

ghost commented 10 years ago

Based on step 1, how could cmst be running in the background if I logged out of X? I'm not aware this still can run like a dameon in the background, I assumed it's a QT gui based app that needs X, so when I log out of X I'm assuming it's shutdown?

So again, I log out of i3 to the tty/console and I upgrade cmst to the latest version, then log back into i3, and cmst is started from the ~/.i3/conf;

exec --no-startup-id cmst -c -m

ghost commented 10 years ago

Ok it started out a better but then I was able to get it to give me a popup message, forget which one, and then it crashed and took connmand off and I had to restart it.

This is what I did.

  1. Upgraded cmst and deleted the ~/.config/cmst directory
  2. logged into i3 and played with clicking the wired/wireless on and off, so far good.
  3. Then I clicked Retain Settings & Notifications
  4. On Off seemed to be working, then I clicked the Notifications off, tried again, then click on, this time it started acting up and crashed again.

So it seems like playing with checking and unchecking the Nofitications preferences is what is triggering this for me.

andrew-bibb commented 10 years ago

I really appreciate you helping me with this, I know it must be a pain.

I think this has to do with asynchronous nature of the dbus reply messages. It probably has been present for a while, but we are only now seeing it because the notifications code is changing the timing. The double dialogs you reported is kind of a clue to that, and the lines I commented out last night really only changed the timing, they had nothing at all to do with dbus calls or replies. I've got a better idea as to where to look now so I'll keep at it.

The startup line looks fine. I was wondering (grasping at straws more likely) if somehow a piece of the program was still running, but not with that startup line.The QT gui part is correct. I'm actually not completely clear on how QT deals with programs running in the systemtray. The tray is for daemon like programs, and the class inheritance tree for the systemtrayicon is, lets just say, different.

ghost commented 10 years ago

Hey it's all good, I'm having fun!

Doesn't seem to be anyone else standing in line, LOL...

I'm honored to be your BugHunter...

I just can't wait to see it all nice and shiney and done... :)

Cheers

andrew-bibb commented 10 years ago

I've done a fairly major rework of my slots to catch the dbus signals. New version just uploaded. This is still a work in progress but I'm hopeful you'll at least be able to load it and run.

I know that if you turn off the technologies and then turn them back on that the tray icon notification popup will have a blank space where the connection name should be. Coding that back in will be an effort, and if we can get this to work without taking down dbus I'll start on that.

ghost commented 10 years ago

It's taking more playing to get it to crash but sooner or later, clicking the preferences popup & notfications on and off, and then clicking the wired on and off several times I was finally able to get it to crash, kill connmand and give me a popup message;

http://i.imgur.com/CPnVLGs.png

Also now when I'm connected the sys tray icon appears as disconnected, the 'connect_no.png' instead of the connected.

andrew-bibb commented 10 years ago

Oddly enough this is good. This error dialog means we still have connection to connman on the system bus. In fact the dialog is a result of a function to handle errors which the API says you should implement when you communicate with connman. We can only ever see that dialog if we actually still have the connection. Never was able to test it as I could never find a way to force an error. Nice to know that it works, but I see I need to pretty up the dialog a bit.

Still looks like a timing issue, but I think we are close. I do know I can slow the user down to ensure we get the dbus replies before we allow another message to be sent. Because of the timing the program is out of sync with the actual state of connman so who knows what will happen when you push a button.

ghost commented 10 years ago

But if I remember correctly when I clicked ok on the popup connmand was killed and I had to restart it.

andrew-bibb commented 10 years ago

Possible, after the error message I kill the interface (line 686 in controlbox.cpp in the current upload).

I just uploaded a version that disables input controls while dbus/connman are thinking about life. I've toggled on and off with and without the notifications checked and it seems good. It is interesting that sometimes there is a noticable lag before connman returns, other times almost instantly.

I've got one more signal to catch properly, and the fact that it is not properly done now may be why you got the crash last night. This one will take me a bit, the function signature is really nasty.

ghost commented 10 years ago

I only had Retain Settings checked and I clicked the wired on and off and sooner or later I could get it to crash again with a DBUS message, and taking connmand offline too, and now the Technologies section would grey out and lock up so you couldn't even click anything in it.

http://i.imgur.com/y919lsq.jpg

Sys Tray icon also still appears unconnected...

andrew-bibb commented 10 years ago

OK. I'll pull out the disable input widgets code. Was not really happy with the way it worked anyway.
I don't quite see how were getting the error message that is appearing in the dialog. We obviously are, I need to try and figure out the "how" part.

Spent hours last night trying to decode the dbus signal signature for services, and getting no where. I'm considering a complete rewrite of my dbus Manager interface from the ground up. I knew nothing of dbus when I started this (and the Manager interface was the first thing I did), but have learned a bit along the way. The announced nightly uploads are probably going to taper off for a bit, either way I've got a huge amount of code to sort through. There will probably still be uploads since this is a handy place to back up my work, but there is likely to be a lot of debug code left in.

andrew-bibb commented 10 years ago

Well I lied. Yet one more version uploaded. I could reproduce the lockout with only Retain Settings checked. If you could can you try the latest. I removed the grey out locking code.

Still need to hack through that function signature or rewrite the entire thing, haven't decided yet which way to go.

I did once or twice get the error dialog you got, but now if you click OK it recovers, or at least it does at my end.

ghost commented 10 years ago

Nope stilll getting popups, got these three;

There was an error reading or parsing the reply from method connman.Manager.GetTechnologies There was an error reading or parsing the reply from method connman.Manager.GetServices. There was an error reading or parsing the reply from method connman.Manager.GetServices.

Then the wired/wirelss on/off buttons dissappeared and it crashed connmand and I had to restart it too...

andrew-bibb commented 10 years ago

OK, code is all back together again. Want to try out the latest version? I sure hope it works because I'm plumb out of ideas.

ghost commented 10 years ago

Hey Boss,

Sorry still crashes and takes connmand offline and the system tray icon shows disconnected again when connected.

All I can say is there is no rhyme or reason here, I just kicked it hard like a dirty dog, LOL, checked and unchecked preferences over and over, and clicked and clicked and clicked the buttons on and off and sooner or later POW it crashes again...

http://i.imgur.com/q1Miy5E.jpg

After it crashed I restarted connmad and then I logged out of i3, when I logged back in the system tray icon wouldn't appear, and I logged in and out a few times but it still wouldn't reappear, so I restarted connmand over a second time to finally get it to reappear...

andrew-bibb commented 10 years ago

I've updated with a few modifications. Probably can still be made to crash, but I'm thinking it may be time to move on to actually getting the notifications working. That is unless looking cross eyed at it makes it crash, then we'll have to deal with it. At some point we've got to decide we can't protect people from doing really foolish things.

I'm actually quite pleased we took this detour, the code on several levels is much more professional now. I had a pretty ratty hack originally and now that I'm dealing with each signal the way you are supposed to I'm a lot happier with it. I also found a few other code things I've fixed up, probably not causing the problem, but at the same time they were not right.

Onward to the notification code.

ghost commented 10 years ago

Yes it still crashes, but sounds good....

Onwards and Upwards... LOL...

andrew-bibb commented 10 years ago

I just uploaded code that should allow basic text notifications to the notify server (if one exists). If you want to test it out let me know what you think. If we are on the right track I'll move on to seeing what I can do with icons.

andrew-bibb commented 10 years ago

Just uploaded another version where I think icons will display.

I'm testing against Dunst and that does not support icons, therefore the weasel word "think" in the first sentence. Really need feedback from someone with a notifcation server that supports icons.

WhyNotHugo commented 10 years ago

I use dunst as well, so I can't say about the icons :( The rest does work fine though.

The Preferences panel says mention Dunst, but that's not precise: any notification daemon should work if using the notification-daemon spec.

Also, the text seems to be cut off, and not wrapping properly: 2014-07-06-210500_1440x900_scrot

andrew-bibb commented 10 years ago

The notifiations panel mentions Dunst because that is what you are using (saw your 6/24 post on the Dunst thread over on Arch, one of the reasons I decided to test with it) . I call the GetServerInformation() method and then output the results of that call to the label. If we can't find or connect to a server the message will reflect that.

Word wrap - it is going to be the death of me on this project. Thanks, I'll fix it.

WhyNotHugo commented 10 years ago

The notifiations panel mentions Dunst because that is what you are using (saw your 6/24 post on the Dunst thread over on Arch, one of the reasons I decided to test with it) . I call the GetServerInformation() method and then output the results of that call to the label. If we can't find or connect to a server the message will reflect that.

Oh. That's extremely friendly! ^_^ I'm not totally convinced that the wording is clear enough though.

Word wrap - it is going to be the death of me on this project. Thanks, I'll fix it.

I don't recall word wrapping being such a pain on qt. Thanks, btw. :)

andrew-bibb commented 10 years ago

Think you are right on the wording. Probably should say something like "Notification server xxxx detected on this system...." Also if we can't find or connect to the server the check box is greyed out.

Word wrap in QT is very easy, I just keep forgetting to do it.

ghost commented 10 years ago

Ahh notifications are working, but there are no icons appearing, if theyr're suppose to?

http://i.imgur.com/AWSHeda.png http://i.imgur.com/mv69HnW.png

By the way I'd considering changing the wording too as it pertains to each technology, like;

Wired Network
Establishing Connection...
Wired Network 
Connection Established
Wifi Network
Establishing Connection...
Wifi Network
Connection Established

But since connman deals with other technologies, I wonder if you'd name them all too, like for the P2P; P2P Network - Establishing Connection... Not sure what other tehcnologies get connected that you might name them too.

Or just a general name for everything like;

cmst Establishing Connection... cmst Connection Established cmst disconnected

Here's some screen shots of wicd with the notifications, if you remember I showed you these before.

http://i.imgur.com/xf8MiDU.png http://i.imgur.com/96eQHRE.png http://i.imgur.com/ox1gCfg.png

By the way I thought dunst just uses notify-send too?

andrew-bibb commented 10 years ago

The notification specification has 3 separate places for text and three separate places to specify images. It is up to the individual server to decide how to display (or ignore) these fields. Basically I need to find the fields that your server uses (without breaking anyone elses). Thanks for the screen shots, that will help with knowing what is being used, and I already see one field that was ignored.

Notify-send is just a way to send messages and convenient from the command line. It is not the only way.

progandy commented 10 years ago

dunst-git supports icons, too. Just support for the file:// protocol is currently missing, a simple absolute path or an icon name work.

By the way, why don't you just supply the icon name without a path?

WhyNotHugo commented 10 years ago

Icon names are recomendable, since they'll use the user's theme. Absolute paths won't, which will make the notifications look rather alien on many desktops.

andrew-bibb commented 10 years ago

Word wrap and the notification server wording in the label have been attended to in the latest (14.07.07-1) upload. I've also added the icon to the image-path hint to see if that helps with the display of these things.

Was not aware of dunst-git and very curious on the URI being the form that does not work. According to the notification specification the image path must be of the URI form (file://) or must be a name in a freedesktop.org-compliant icon theme.

Anyway regarding the paths and names, If this worked the default will show a system icon from the current theme if the program is called with the -i switch and if the icon is available, otherwise it falls back to the hard coded ones. Icon names are used if available (lines 569, 577, etc in controlbox.cpp all grab from the icon theme if available and send to the notifications). Fallbacks are coded in a qresource file.

Right now there are an insane number of conversions: qresource->QIcon->QPixmap->temporary disk file in PNG format. I'm going to clean that mess up once I get the icons to actually display in the notifications.

In Connman getting the actual service that is changing state (ready, online,idle, etc) is not real easy. Getting the overall state is a simple matter, but finding the actual service that triggered that change is going to involve more code than I feel like starting tonight. Long way of saying the two line notifications in the pictures are not quite there yet.

ghost commented 10 years ago

wicd uses libnotify for the notifications...

And I thought the NetworkManger does the same?

andrew-bibb commented 10 years ago

NetworkManager does not surprise me. Not that familiar with wicd to have an opinion one way or the other. I really want to avoid the libnotify dependency, and anything else it needs as well. The specification is pretty simple, although one could argue it needs to be written better. I've pretty much got it all implemented, really just need to determine what the preferred format is from servers for image files (not the client end which I've got worked out). I think I said 3 earlier, there are actually more, several are depreciated but that does not mean they are still not used.

I also want to reuse that code in my media player for playlist change notifications once I get back to that project (ulterior motives and all that)

WhyNotHugo commented 10 years ago

NetworkManager does not surprise me. Not that familiar with wicd to have an opinion one way or the other. I really want to avoid the libnotify dependency, and anything else it needs as well.

I'm curious as what the issue is with libnotify. It provides exactly what you need, with no bloat attached. It'd definitely facilitate development/maintenence.

andrew-bibb commented 10 years ago

Some just to avoid another dependency, mostly it is just me. On the second part about equal portions stubbornness and how I learn things (I'm an engineer in my day job - we revel in taking things apart and putting them back together again - sometimes they even still work afterwards).

A month ago, not being happy with any calculator I could find on the market, I decided to build my own (a WP34S, beyond cool, you should spend some time with Google if you are interested). It is just what I do.

All that being said, if I can't get it working with a reasonable amount of effort I will go over to libnotify, and I'll most likely pull in your fork to do it.

ghost commented 10 years ago

I personally thought libnotify was the standard for notifications in apps that provide it?

As far as another dependency, I see now what you mean, since you can make popups with qt5, I guess if you can just make these qt popups use different qt & gtk themes and use the typical icons you see that networkmanager uses when connecting, disconnecting, etc., like the screen shots I showed, then why not... :)

andrew-bibb commented 10 years ago

Libnotify probably is the standard for sending notifications. I know if I had not spent months hacking through the Connman DBUS stuff I would not have tackled it. It absolutely would not have been worth the effort, but after Connman the Notification specification was a walk in the park. It is only 251 SLOC, and probably one third of that is just convenience functions to make my life easier.

I'm only talking the transport mechanism. To theme or not to theme, show icons or not, format text, etc. is all handled by whichever notification server you have installed on your system. I'm not touching that in any way. All I'm using QT for is to transport the data across DBUS. What happens after the data is delivered is not my concern.

edit 1: Just though of this, if I do use libnotify we either force people to have it as a dependency (I agree it is not large at all, but it is another dependency), or I need to code the main program to load and use it as an optional plugin. At this point I'll take the 251 SLOC hit, plus I'm stubborn.

andrew-bibb commented 10 years ago

Just uploaded a version starting to cleanup the notify code. Can you see if icons work with this one? If they work at all I'd expect system theme ones to have a better chance than the hardcoded ones, although if system theme ones work I'd appreciate it if you could also check to see if the hardcoded do as well.

The two line notification messages should also be functional (they actually were before, I just was not sending 2 lines of text).

There is a problem if you start with no services active and then if you activate one the notify code will not be called. If you start with services active and then disactivate them all is fine. I know what it is and how to fix it, just not tonight.

andrew-bibb commented 10 years ago

Think we may be there, just uploaded 14.07.10-3. I installed xfce4-notifyd which supports icons and the fallback icons are displaying in the notify popup. Can someone with an icon theme check to see if the theme icons display. Thanks.

The problem described in the post immediately above has been fixed as well.

WhyNotHugo commented 10 years ago

Text wrapping seems to work fine now! :D No icons with xfce4-notifyd. I'm using cmst-git. :(

Icons work with xfce4-notifyd on other apps.

ghost commented 10 years ago

For libnotify it's probably good to make it a optional requirement, so if people don't want it, they can disable it at compile time...

In the latest update to cmst, I get both the hardcoded wired and wifi icons appearing, but now there's no wifi system tray icon, see #22...

I'm using; xfce4-notifyd-0.2.4

andrew-bibb commented 10 years ago

hobarrera, Just to be sure, with xfce4-notifyd you were getting the text part but not the icon part of the notification? If yes one other question, are you starting with the -i switch to use system theme icons?

I'm a bit at a loss as to why it was working for moulei and I, but not you. Trying to gather as much information as I can.

andrew-bibb commented 10 years ago

I put xfce4-notifyd back on my system and I'm seeing notifications and icons just the way you are supposed to. I'd like to close this one out if we are done so is the consensus that icons work, and the two line notification text being sent is acceptable? If not what else needs to be done?

WhyNotHugo commented 10 years ago

It works now. I must have done something wrong when testing last time. Sorry about that.

WhyNotHugo commented 10 years ago

Some of the notifications appear on the center of the screen instead of the upper-right corner. That's an xfce4-notifyd issue, right?

andrew-bibb commented 10 years ago

hobarrera, I've noticed the occasional center of the screen popup as well. I think is is an xfce issue as there is nothing in the freedesktop Notification specification that tells where to position the message. The whole idea is that we (or libnotify) send only data to be displayed, how or where it is displayed, or even if it is displayed is up your notification server and how you configure it.

andrew-bibb commented 10 years ago

Right now this one seems pretty well wrapped up so I'm going to close it down.