Add groundwork for Controller COs #2884

Holzhaus · 2020-06-18T17:19:14Z

This add a group attribute to each controller, so that we can create COs for each controller. This group is passed as third parameter to the script's init function, so that controller-specific COs can be access from the controller itself.

Right now, this adds a reload_scripts CO for each controller. I know if this is actually useful, but in theory you could actually reload the controller script using a button press on the controller itself (although you should only need this if your controller script is buggy). I mostly implemented this as a Proof of Concept and can remove this if you want.

The actual reason why I implemented this is because I really want to have MIDI Clock support at some point, and one prerequisite is controller-specific COs to toggle sync lock (and disable/enable MTC output). So if I press the SYNC button for the drum machine on my controller, it would then set [PortMidiControllerN],sync_enabled to 1 and Mixxx could start sending/receiving MIDI timing clock signals.

daschuer

Can we register the controller as a midi clock consumer by a dedicated function in the JS API?

However we actually need such "controller specific namespace" for controller dependent skins and settings. Here the Pug And Play topic essential to match the stored preferences to the right controller after restart.

I think if we consider that in the group name as well it becomes a nice solution.

daschuer · 2020-06-18T22:11:13Z

src/controllers/bulk/bulkenumerator.cpp

@@ -56,8 +57,13 @@ QList<Controller*> BulkEnumerator::queryDevices() {
                continue;
            }

+            // Generate a group for this controller
+            QString group = QStringLiteral("[BulkController") +
+                    QString::number(index) + QStringLiteral("]");


This can be just a QChar(']');

Holzhaus · 2020-06-19T13:15:08Z

Can we register the controller as a midi clock consumer by a dedicated function in the JS API?

I think we could, but it should also be possible to sync from a midi device, and for that we also need all the other sync-related COs I think (e.g. sync_master, etc.). Or what do you have in mind?

However we actually need such "controller specific namespace" for controller dependent skins and settings. Here the Pug And Play topic essential to match the stored preferences to the right controller after restart.

I think if we consider that in the group name as well it becomes a nice solution.

That's the hard part though. I have no idea how to match devices after a restart. Is this even possible with portaudio? We could use the device name, but would that work if I have 2 identical devices attached, e.g. 2 Allen & Heath Xone:K2's?

daschuer · 2020-06-19T18:35:51Z

Ah, now I understand. I did not consider the case where a user can switch between different external midi clocks. Does this actually exist? I don't know one.

In this case I think it is better to have it more generic like [ExternalClock1] [ExternalClock2]
This way we can build the skins around it easily.

This way we can build a GUI around it and the External clock sources like Albaton Link OSC and midi can subscribe to one of them.

Be-ing · 2020-06-21T05:56:48Z

My first thought was the same as @daschuer that a JS API may be more appropriate for MIDI clock. I don't fully understand what you have in mind for the design of using MIDI clock, so it's difficult to understand where you are going with this. I think we should have a larger discussion about how MIDI clock should work in Mixxx so we know what it will require before proceeding with this.

Also, let's not add any features to the legacy controller scripting system and only add new features when using JS modules.

uklotzde

Just some minor comments.

Update: Ignore if this PR might become obsolete. I didn't notice.

uklotzde · 2020-06-21T12:21:43Z

src/controllers/controller.cpp

          m_bIsOutputDevice(false),
          m_bIsInputDevice(false),
          m_bIsOpen(false),
          m_bLearning(false) {
    m_userActivityInhibitTimer.start();
+
+    m_pReloadScripts = make_parented<ControlObject>(ConfigKey(group, "reload_scripts"), this);


QStringLiteral("reload_scripts")

uklotzde · 2020-06-21T12:23:06Z

src/controllers/controller.h

 class ControllerJSProxy;

 class Controller : public QObject, ConstControllerPresetVisitor {
    Q_OBJECT
  public:
-    Controller();
+    Controller(const QString& group);


explicit

Minor improvement: Since Controller is supposed to be an abstract base class we could reduce the visibility of the ctor from public to protected.

uklotzde · 2020-06-21T12:24:56Z

src/test/controller_preset_validation_test.h

@@ -23,7 +23,7 @@ class FakeControllerJSProxy : public ControllerJSProxy {
 class FakeController : public Controller {
    Q_OBJECT
  public:
-    FakeController();
+    FakeController(const QString& group);


explicit (although just a test class)

uklotzde · 2020-06-21T12:28:24Z

src/controllers/controller.h

@@ -64,6 +66,10 @@ class Controller : public QObject, ConstControllerPresetVisitor {
    inline const QString& getCategory() const {


We could remove all those redundant inline keywords while editing this file.

github-actions · 2020-10-19T00:22:00Z

This PR is marked as stale because it has been open 90 days with no activity.

Holzhaus · 2021-03-14T01:27:06Z

Superseeded by #3703. I cherry picked some commits.

Holzhaus added the controller mappings label Jun 18, 2020

Holzhaus added 3 commits June 18, 2020 22:21

controllers: Add group for each controller

715e782

controllers/engine/controllerengine: Pass controller group to init()

7a57f02

controllers: Add CO to reload scripts

47b14dd

Holzhaus force-pushed the controller-cos branch from 31d346f to 47b14dd Compare June 18, 2020 20:21

daschuer approved these changes Jun 18, 2020

View reviewed changes

controllers: Use QChar instead of QStringLiteral

f06786d

uklotzde reviewed Jun 21, 2020

View reviewed changes

github-actions bot added the stale Stale issues that haven't been updated for a long time. label Oct 19, 2020

Be-ing changed the base branch from master to main October 23, 2020 23:16

Be-ing marked this pull request as draft October 23, 2020 23:16

Holzhaus mentioned this pull request Feb 8, 2021

MIDI Clock Input #702

Closed

Holzhaus closed this Mar 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add groundwork for Controller COs #2884

Add groundwork for Controller COs #2884

Holzhaus commented Jun 18, 2020

daschuer left a comment

daschuer Jun 18, 2020

Holzhaus commented Jun 19, 2020

daschuer commented Jun 19, 2020

Be-ing commented Jun 21, 2020

uklotzde left a comment •

edited

Loading

uklotzde Jun 21, 2020

uklotzde Jun 21, 2020

uklotzde Jun 21, 2020

uklotzde Jun 21, 2020

github-actions bot commented Oct 19, 2020

Holzhaus commented Mar 14, 2021

		@@ -64,6 +66,10 @@ class Controller : public QObject, ConstControllerPresetVisitor {
		inline const QString& getCategory() const {

Add groundwork for Controller COs #2884

Add groundwork for Controller COs #2884

Conversation

Holzhaus commented Jun 18, 2020

daschuer left a comment

Choose a reason for hiding this comment

daschuer Jun 18, 2020

Choose a reason for hiding this comment

Holzhaus commented Jun 19, 2020

daschuer commented Jun 19, 2020

Be-ing commented Jun 21, 2020

uklotzde left a comment • edited Loading

Choose a reason for hiding this comment

uklotzde Jun 21, 2020

Choose a reason for hiding this comment

uklotzde Jun 21, 2020

Choose a reason for hiding this comment

uklotzde Jun 21, 2020

Choose a reason for hiding this comment

uklotzde Jun 21, 2020

Choose a reason for hiding this comment

github-actions bot commented Oct 19, 2020

Holzhaus commented Mar 14, 2021

uklotzde left a comment •

edited

Loading