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

Dmxkeypad #280

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

Dmxkeypad #280

wants to merge 27 commits into from

Conversation

kripton
Copy link
Contributor

@kripton kripton commented Apr 20, 2014

Adds a DMX KeyPad to the simple console allowing ranges of channels to be manipulated at once and it's easier to use on a touchscreen than faders.
I don't consider this feature-complete but it should be production-ready. All commands that can lead to strange behaviour (crashes or 100% CPU load) should be impossible due to buttons beeing disabled.

TODO:

  • documentation
  • add it to the webserver somehow (together with an "clear universe" button as we have on the simple console

Discussion thread: https://sourceforge.net/p/qlcplus/discussion/development/thread/6039d20d/

EDIT (janosvitok): discussion thread in the new forum: http://www.qlcplus.org/forum/viewtopic.php?f=12&t=3091

@janosvitok
Copy link
Contributor

I rebased the commits to current master (to get rid of the merge commits): https://github.com/janosvitok/qlcplus/commits/pr/280

I did some quick tests. It looks nice and useful.

Bug: Pressing FULL several times after "1 THRU 10 AT 0 THRU" each time adds AT

@matthijskooijman
Copy link

@janosvitok if you force push your rebased work into the branch used to create this PR, this PR will be updated automatically.

@edogawa23
Copy link
Contributor

edogawa23 commented Jul 26, 2020

I wonder if there still is interest in getting this completed and included/merged. I realize that it's unfinished, but still very useful even in this state, in my opinion.

I have looked at kripton's branch and manually merged it to a new branch in my own fork, to get a rebased version. I'm sure there is a more elegant method to do that, but I still have a hard time getting along with git and github and could not find one. So I apologize beforehand if this makes things more complicated. Here is the link.

The patch is small after all, two new files, a few changes that had trivial conflicts that needed to be resolved(because kripton's branch hasn't been rebased in a long time) for simpledesk.cpp/h, and the qmake src.pro file.

So today in the morning I had a go on adding keyboard input fo the DMX keypad and got it working after ~ 3 hours without much hassle. Here's my mapping:

Channel/value digits: main or numpad digit keys
+: main or numpad "+"
-: main or numpad "-"
AT: "a"
CLR: "Escape" key
THRU: "t"
FULL: "f"
BY: "b"
Enter: "Return" or "Enter"

Also as a rather unexperienced coder I don't fully understand the code yet, especially that state machine part of it. I see that when a command is considered complete (after pressing ENTER or FULL), the current channel selection is cleared, but I cannot understand why (yet). It would be great to be able to keep the selection and further modify the value until CLR (or a new button for that purpose) is being pressed.

I have used a few hardware consoles and thus can think of several more improvements, but for now I want to find out if there's a chance to "revive" this PR (and interest of kripton, janosvitok or whoever might be able to contribute...)

@mcallegari
Copy link
Owner

I kept this on hold basically because I planned this feature for QLC+ 5 and didn't want to invest effort on v4.
Seeing how it is going (my bad...) I can dedicate some time to this and bring it to a usable state.
Dunno when though :(

@yestalgia
Copy link
Contributor

With the keypad being a more promenent mode of input in QLC+ 5, is it worth enabling this to get users used to the concept?

@edogawa23
Copy link
Contributor

I have the extended patch from above ready to apply on top of current QLC+ v4 GIT, but as Massimo seemed to oppose to adding it to v4, I didn't even try and pull request it. For me it's really useful in theater during setting up/testing/focussing and even while writing cues for a new lighting design, I have used it for a while and haven't noticed any bad side effects.

See here, I've just pushed this to github again, on top of current master.

@kripton
Copy link
Contributor Author

kripton commented Nov 14, 2022

Thanks for bringing this up again. Since QLC+5 has a keypad by now (see https://github.com/mcallegari/qlcplus/blob/master/qmlui/qml/KeyPad.qml), I would propose to sync the two implementations we have now. Either we adapt this PR to match the code of what's inside QLC+5 or we change QLC+5's implementation to what is found here. I would assume the first approach has the higher probability of getting merged.

@mcallegari
Copy link
Owner

mcallegari commented Nov 14, 2022

Technically, the only issue here is on macOS, where Simple Desk has serious horizontal scaling issues.
Roadmap-wise instead, I confirm that I would prefer to have this as a new feature in v5.
Integrating this PR would give one more excuse to v4 people to not even try v5.
Right now I received useful feedbacks on v5 from one single QLC+ user - repeat - one single user.
To me this means: nobody uses it. It's really depressing.

In any case, there is a KeyParser in the engine now:
https://github.com/mcallegari/qlcplus/blob/master/engine/src/keypadparser.cpp
and test unit:
https://github.com/mcallegari/qlcplus/blob/master/engine/test/keypadparser/keypadparser_test.cpp

So yes, to guarantee a consistent supported syntax this PR should be migrated to using the engine parser first.

@yestalgia
Copy link
Contributor

This seems like a sensible way forward.

I'm going to be dedicating more time to helping out on this project in the coming months including switching to +5 in my professional capacity. Money is one thing but I know it will help more for me to dive in and get my hands dirty.

Don't feel discouraged Massimo! It's a big change but it will be worth it for sure. I'm seeing the same issue happening in GrandMA 2 vs 3 but people are coming around.

Thanks for the discussion.

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.

6 participants