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 Request] Popup on changing the set #122

Open
vguttmann opened this issue Dec 11, 2020 · 21 comments
Open

[Feature Request] Popup on changing the set #122

vguttmann opened this issue Dec 11, 2020 · 21 comments
Labels
enhancement New feature or request

Comments

@vguttmann
Copy link

vguttmann commented Dec 11, 2020

Is your feature request related to a problem? Please describe.

There is no indication on which set is active when I change them. Controllers already have few keys, so changing presets should only occupy one button, but this means that I always need to remember the set I'm in, or I'll loose orientation.

Describe the solution you'd like

Some kind of popup or notification (that works in fullscreen as well) and shows the number/name of the set that was changed to, potentially using a normal notification on, or such a screen popup like for display brightness, volume and such on ubuntu.

Describe alternatives you've considered

I thought about doing it myself, but frankly, I would either break something or not do it properly. Don't get me wrong, I'm not tech-illiterate (if I were, I wouldn't be using Linux), but I have other stuff that demands my attention

Additional context


Upvote & Fund

  • If you find this issue important, mark it with 👍. It lets us see which fixes and features are demanded by the most users.
  • We're using Polar.sh so you can upvote and help fund this issue. It may incentivize some developers to contribute to this project and fix some bugs.
  • Funded developer receives the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@pktiuk pktiuk added the enhancement New feature or request label Dec 13, 2020
@pktiuk
Copy link
Member

pktiuk commented Dec 13, 2020

I think it would be a nice feature. I will investigate it later.

@vguttmann
Copy link
Author

vguttmann commented Dec 18, 2020

I think I might able to do that. It would be helpful if you could tell me which script ultimately handles set changes, as I could avoid digging through tons of grep output.

@vguttmann
Copy link
Author

vguttmann commented Dec 18, 2020

Okay, I got a first version working (although it's pretty dirty and inelegant): I execute the command notify-send "AntiMicroX" "Set n is now active" -i "<absolute path to the icon>" after changing the set using the advanced menu. To implement it properly however, I would need to know what handles the button presses/set changes

@pktiuk
Copy link
Member

pktiuk commented Dec 18, 2020

I am not sure about this way of calling notification.
I think we should use system-independent way of creating notifications to avoid creating new problems with: #4

The best option would be using qt-based notifications, because it wouldn't change our dependencies, and it would work on all systems.

@pktiuk
Copy link
Member

pktiuk commented Dec 18, 2020

In terms of place where set is changed you can find it there:
/antimicrox/src/joytabwidget.cpp:1223

@pktiuk
Copy link
Member

pktiuk commented Dec 18, 2020

@vguttmann
Could you investigate the possibility of using system-independent way of publishing notifications before you start work?

If there are none we could think about adding notifications as a linux-only feature.

@vguttmann
Copy link
Author

I will have a look for libraries that can do that, I'm sure that there's something out there.

@vguttmann
Copy link
Author

So, after taking a look, I found something in QT called QSystemTrayIcon, which seems to be what we want. I hope to have implemented it until the weekend.

@vguttmann
Copy link
Author

vguttmann commented Dec 23, 2020

Although there is the possibility to add notifications on all platforms through QSystemTrayIcon::showMessage, I definitely need to read up on my C++. I am not sure how long it will take though, but right now I am optimistic that it will go fairly well.

@pktiuk
Copy link
Member

pktiuk commented Feb 7, 2021

I think it would be better to notify users about changing sets using default linux notifications. Just like it is done here: #146

@vguttmann
Copy link
Author

vguttmann commented Feb 7, 2021

Yep, and it would certainly be a LOT easier. I have been tinkering for a long time now, but haven't gotten something to work yet which uses only Qt. With send-notify, I can implement that in a day or so (including setting up git once again because I had to do a clean reinstall). A day sounds pretty bad, but this would be time from reply to PR.

@pktiuk
Copy link
Member

pktiuk commented Feb 7, 2021

I will try to deal with it now. (or at least today)
Just tell me where is a good place in code to put this notification (in your opinion, if you remember where you looked). :D

@vguttmann
Copy link
Author

vguttmann commented Feb 7, 2021

Just wait, I'll do it, I think I still have that figured out. Just tell me which icon to use. You're managing the project, and shouldn't do all the work.

@pktiuk
Copy link
Member

pktiuk commented Feb 7, 2021

Ok

I think you could use input-gaming, applications-games or simply antimicrox icon for this purpose.

@pktiuk
Copy link
Member

pktiuk commented Feb 7, 2021

Remember also that this implementation of notifications should be compiled only on UNIX-based systems.

@vguttmann
Copy link
Author

notify-send has one crucial limitation: If I want it to display notifications in full screen, I would have to use "urgent" as the importance, which then results in the message not clearing itself. Normal and low urgency do, but they don't work in full screen

@vguttmann
Copy link
Author

vguttmann commented Feb 8, 2021

Otherwise I have it finished, I could submit the pull request. The command only gets sent on Linux (I assume so, I don't have a windows machine handy). notify-send has no configurable timeout (it has a timeout flag, but that one is disfunctional due to a feauture, ykwim, and the maintainer has no intention of changing it.

I did implement it in joytabwidget.cpp

Also, I have not implemented a way to disable them.
@pktiuk

@pktiuk
Copy link
Member

pktiuk commented Feb 8, 2021

The command only gets sent on Linux

In this case simple #ifdef Q_OS_UNIX should be enough

I could submit the pull request.

You can open PR marked as a work in progress. (doing it in this way makes monitoring progress easier, and allows spotting errors earlier)

Also, I have not implemented a way to disable them.

You can to it just like I did it here: https://github.com/AntiMicroX/antimicrox/pull/146/files

@vguttmann
Copy link
Author

vguttmann commented Feb 9, 2021

Well, I got it working, but I had some fights with git, and GitHub complains about code formatting.

What the hell? Git inserted stuff about the commits into the code. I don't blame git for that, but I have no idea how I did that.

@vguttmann
Copy link
Author

Github still complains about code formatting, but at least I have no failed builds now. It would be nice if you could take a look at it.

@pktiuk
Copy link
Member

pktiuk commented Feb 9, 2021

I have reviewed your change. I have also described a way of dealing with formatting code. Just check in PR.

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

Successfully merging a pull request may close this issue.

2 participants