mcallegari / qlcplus

Q Light Controller Plus (QLC+) is a free and cross-platform software to control DMX or analog lighting systems like moving heads, dimmers, scanners etc. This project is a fork of the great QLC project written by Heikki Junnila that aims to continue the QLC development and to introduce new features.
Apache License 2.0
921 stars 349 forks source link

Highlight Fixtures feature #1506

Closed Timebutt closed 1 month ago

Timebutt commented 6 months ago

This PR introduces a new feature into QLC+: the Fixture Tester. With this feature (that you can toggle) with a button in the Fixture Manager selected fixtures will output white light. This is very similar to Chamsys MagicQ's head test, and allows a user to quickly verify a fixture is patched correctly. Walking through the list of all patched fixtures highlights issues in patch. Selecting groups of fixtures will show you if maybe you missed a fixture.

@mcallegari this is my first real feature I add to the codebase, I'm not sure if you can take a quick look and see if I'm heading in the right direction? There's quite a few things I want to add in (the feature is not working for fixtures that require the shutter to be in a certain position, like a Martin Quantum). Selecting groups is also I'd like to add, ... I'd rather get early feedback than finish the entire thing, only to have to redo a lot of it for instance.

I'll provide documentation once the initial implementation is done and before merging obviously ;)

coveralls commented 6 months ago

Coverage Status

coverage: 31.905% (-0.06%) from 31.964% when pulling ea23d00ec9353aba96620a9df89e54f8fc0377af on Timebutt:add-fixture-tester-feature into 32fc8094e15028adb28c29e58de2d96461a3405b on mcallegari:master.

yestalgia commented 6 months ago

G'day @Timebutt and thanks for this,

This feature sounds similar to the new "Highlight" feature which has been implemented in v5. Perhaps this name is more suitable?

It's important that people don't confuse this feature with the creation of scenes or running the show live. I wonder if it might make sense to automatically "Un-highlight" the fixtures when moving away from the fixtures panel.

The "Highlight" icon in V5 is a lightning bolt, would you be comfortable with me changing it to this? If so, give me access to the branch and I'll happily make the change before this gets merged.

But yeah - a great idea!

Cheers.

Timebutt commented 6 months ago

Hey @yestalgia, thanks for getting back to me so quickly! I updated a few things already following your suggestions: I renamed the feature to Highlight Fixtures to be in line with what v5 will bring. I didn't know a similar feature was already implemented, and would have taken a look had I known ;)

I also updated the code to indeed only highlight fixtures when a user is currently in the Fixture Manager tab, this would lead to very confusing issues otherwise. It's one of the things that was on my to-do list ;)

With regards to the icon: can you point me to that specific lightning bolt? I went through the existing icons but can't seem to find that specific one. I don't know where all QLC+ v5 code is for that matter, is it in a specific branch I am missing?

mcallegari commented 6 months ago

This is the commit https://github.com/mcallegari/qlcplus/commit/8d91944f457073ca1d2a761dc89e0af50b46b481 and this is the icon, displayed using FontAwesome https://fontawesome.com/v4/icon/bolt

mcallegari commented 6 months ago

@Timebutt I posted the commit so that you can look at the current code and see how I implemented it in v5. You would have found that 2 commits later I separated pan and tilt into another feature. 3d891a8f3c7c8d2bf6b8627f2fac77a166e51288

mcallegari commented 6 months ago

Please review your code to stick with the current code style.

Timebutt commented 6 months ago

Hey @mcallegari thanks for the thorough review! I'm not entirely done implementing the feature yet and was hoping to do those things once I wrap things up, but your remarks greatly help adopting the code style in this repository ;) I'm used to codebases that rely entirely on Prettier for automated formatting, as it's a great way to keep code style consistent. Don't worry though, I'll address all code style issues.

Thanks for the commit you referenced, that helps me implement opening the shutter for more complex fixtures and was the next item on my list. I'm just copying your implementation and adding it in. I'll fix everything you mentioned and let you know once I think I'm done implementing so you don't have to review multiple times?

mcallegari commented 6 months ago

If this is a WIP and doesn't yet require a code review, then set the PR as draft (this time I did it for you) Automated code style review is in progress: #1427

Timebutt commented 6 months ago

Ah I see you use drafts, my bad and noted thanks! I'm just struggling with this last thing: I'm trying to make sure multi-head fixtures also highlight correctly and as such I implemented this loop.

for (int i = 0; i < fxi->heads(); i++)
    {
        QLCFixtureHead head = fxi->head(i);

        quint32 whiteChannel = head.channelNumber(QLCChannel::White, QLCChannel::MSB);
        quint32 redChannel = head.channelNumber(QLCChannel::Red, QLCChannel::MSB);
        quint32 greenChannel = head.channelNumber(QLCChannel::Green, QLCChannel::MSB);
        quint32 blueChannel = head.channelNumber(QLCChannel::Blue, QLCChannel::MSB);

        if (whiteChannel != QLCChannel::invalid())
        {
            source->set(fxi->id(), whiteChannel, 255);
        }
        else if (redChannel != QLCChannel::invalid() && greenChannel != QLCChannel::invalid() && blueChannel != QLCChannel::invalid())
        {
            source->set(fxi->id(), redChannel, 255);
            source->set(fxi->id(), greenChannel, 255);
            source->set(fxi->id(), blueChannel, 255);
        }

        quint32 panChannel = head.channelNumber(QLCChannel::Pan, QLCChannel::MSB);
        if (panChannel != QLCChannel::invalid())
        {
            source->set(fxi->id(), panChannel, 127);
        }

        quint32 tiltChannel = head.channelNumber(QLCChannel::Tilt, QLCChannel::MSB);
        if (tiltChannel != QLCChannel::invalid())
        {
            source->set(fxi->id(), tiltChannel, 127);
        }

        quint32 intensityChannel = head.channelNumber(QLCChannel::Intensity, QLCChannel::MSB);
        source->set(fxi->id(), intensityChannel, 255);
    }

I see that you didn't implement this in your current implementation right? When I try to highlight one of my 10x1 sunstrips for instance, only the top bulb would illuminate, making it hard to spot in a large project. My only remaining challenge is now to get the ShutterOpen or LampOn capability for which I am using your code, but mapping it to the individual head channel and I don't yet see how that is mapped. I figured maybe the channelsMap would be that mapping but that doesn't seem to be it. Could you perhaps show me where I can find this mapping, or how I can find the right ShutterOpen or LampOn for a specific head of a fixture?

mcallegari commented 6 months ago

AFAIK fixtures that have LampOn are traditional single head with discharge lamps For each head you can retrieve channels by type. Please take some time to study the QLCFixtureHead code: https://github.com/mcallegari/qlcplus/blob/master/engine/src/qlcfixturehead.h

In general, take some time to study the QLC+ code which is quite complex. It's not just "reaching the goal". It's "reaching the goal considering everything that has been done so far".

yestalgia commented 6 months ago

Hey @yestalgia, thanks for getting back to me so quickly! I updated a few things already following your suggestions: I renamed the feature to Highlight Fixtures to be in line with what v5 will bring. I didn't know a similar feature was already implemented, and would have taken a look had I known ;)

I also updated the code to indeed only highlight fixtures when a user is currently in the Fixture Manager tab, this would lead to very confusing issues otherwise. It's one of the things that was on my to-do list ;)

I find the highlight feature super handy when creating position presets (scenes). I think the highlight being made available when creating a new scene would be marvelous as well.

When creating scenes, it's possible to click multiple fixtures and "Select" or "filter" them. Perhaps the way to go could be to add a button next to the copy/paste values button.

I still like the original idea you had which makes it available for when you're patching fixtures and I still think it should move forward as you've planned it. This would be an addition to your original implementation.

Might be something to consider down the track if more users ask for it.

mcallegari commented 4 months ago

SVG icons are not necessary. In v5 I can also use symbols from FontAwesome. Can you please share a screenshot of where this feature ended up? Thanks

yestalgia commented 2 months ago

@Timebutt thanks for this again. Can you please merge master and push so I can download a build and test it?

mcallegari commented 2 months ago

@Timebutt I think there's something wrong with the master update. This PR says you modified 369 files. Please review

Timebutt commented 1 month ago

There was definitely something wrong. I have no experience properly rebasing forked repositories to an upstream remote and couldn't get it fixed afterwards, should have gone with a simple merge instead. To fix things, I have created a new clean pull request from a branch where I cherry-picked all relevant commits.

I'm closing this pull request since it's no longer relevant!