Controller Shared Data: Initial Proposal

[ywwg]

No description provided.

[daschuer]

Thank you. It looks already good.
I was a bit puzzled with the global name space thing.
At the begining you write it is not at focus. At the end you describe options for it. I like that part.

So you may state it clearly as a "goal" that it is part of the proposal to be extensible for a global namespace for an universal shift solution. But without concider details.

You may also mention keyboard and Skins as another boundary if the new feature.

[JoergAtGithub]

• You should mention that the namespace is already an attribute of the element in each mapping XML.
• Instead of the term "grouping" you should better use a term that sound different to "group". Would "entity" work in English language for this context?

[ywwg]

entity is good

[acolombier]

What is the purpose of adding another dimension here, as opposite to stricly rely on the key, such that (["deck1", "button") would become deck1.button?
I am wondering if this second layer, which would likely induce the need for maps of maps, which could present more challenges in terms of stability and complexity (though performance is likely negligible since I doubt the scale we are talking about here is significant enough!)

Edit: just noticed the triple-keyed map, so I assume we would use a tuple instead, so I assume no impact on stability or complexity! Now just wondering about practicality?

[ywwg]

I see no significant challenges in terms of stability, complexity, or practicality with the extra layer, and lots of benefits. Since the dominant use-case is hierarchical ("deck1","button"), I would prefer to encode that in the design rather than force controller mapping writers to invent their own methods to simulate that structure in a flat object. This also mirrors how we talk about control objects in Mixxx so it's consistent with the pattern we use everywhere in the codebase.

Furthermore on the read side, without the separation of objects every controller mapping would have to manually parse the key to determine which entity and object is being referenced. I prefer to lend them a hand and give them the structure we know will be used, no parsing required. (If they don't need an entity they can use "global" or whatever they want).

What issues of practicality do you see? That's kind of a vague word so I don't know what it's referring to

Managing the nested maps would all be handled in testable C++ code so I am not worried about juggling the maps and signals. The idea is to keep the complexity in C++ and make the javascript simple and predictable.

[acolombier]

What issues of practicality do you see? That's kind of a vague word so I don't know what it's referring to

It's just an additional grouping you need to do, which I don't see the added value for. IMO, having one arbitrary key is always better than two, especially when it would come to debugging/extending the mapping/code of someone else.

The idea is to keep the complexity in C++ and make the javascript simple and predictable.

This is a bit vague, could you clarify how this design make Javascript more "simple and predictable"?
From my perspective I see the opposite, where before you could have one wrong key (e.g problem with interpolation, typos, whatever...).

[daschuer]

Can you give some examples for your pro and cons arguments?

From my experience a hash single hash lookup is faster than three:

QVariant value = m_sharedValues.value(namespace + entity + key);  
QVariant value = m_sharedValues.value(namespace).value(entity).value(key);  

But that is an implantation detail that can be hidden.

The structured version allows nicely to iterated through an entity.
The first one would allow to add or remove layers dynamically.

But which requirements do we have for them?

[acolombier]

I think the idea was to use a single hash with three key, much like your first line, but not sure.

The structured version allows nicely to iterated through an entity.

Would we want to allow an entity to be introspect-able? How would that look? Is this more from a debug/developer tool perfective or from a controller mapping side?
I guess the single key model would also allow that (much like the current developer tool) and could leverage "virtual" layer - a key such as deck1.samplers.slot1 could still be interpreted in a tree structure thanks to the ., without constrain on the number of layer, as opposite the entity/key design)

[ywwg]

let's keep in mind that these are very, very small maps (perhaps dozens of entries), nor do we need to worry about micro latency (buffer-size latency is probably the scale we should worry about: 1ms or more) so I would be shocked if speed of lookup is a concern.

I am looking at the mapping for the S4 and almost every single data update can be expressed as "entity:key:value" -- notably, the current mapping inverts this in a way that is inconsistent with most of mixxx, doing "key:entity" (padsmode:deck.group = value) rather than the more familiar (deck.group:padsmode = value) which is exactly the sort of wheel-reinvention I want to avoid.

I think it's easy to argue for fifteen different ways of designing this structure, which is why I'm very specifically choosing to be consistent with how Mixxx designed ControlObjects rather than try to invent something new.

[acolombier]

so I would be shocked if speed of lookup is a concern.

I agree that looking at the scale of a namespace, the hashing performance is probably anecdotal.

I am looking at the mapping for the S4 and almost every single data update can be expressed as "entity:key:value" -- notably, the current mapping inverts this in a way that is inconsistent with most of mixxx, doing "key:entity" (padsmode:deck.group = value) rather than the more familiar (deck.group:padsmode = value)

I totally agree that, for the current S4 implementation, entity:key works well, and it’s practical for what we have now. But as we look ahead—especially with plans for deeper screen interaction and library integration—this approach might introduce some ambiguity. For example, how would we handle the case like padsmode:deck.group? Would it be entity=padsmode, key=deck.group, or entity=deck.padsmode, key=group? If we don’t standardise this now, we risk creating inconsistencies that could confuse contributors down the line, but also, standard can often be perceive a barrier to entry/painful devx. IMO, this is why using a single flat key would be easier.

Now, there is a risk than once mapping uses padsmode.deck.group and another one deck.padsmode.key, but at least, this would be one single symbol (either a variable or a literal)

which is exactly the sort of wheel-reinvention I want to avoid.

Yes, I share the exact same interest here - I have the feeling that this "entity" concept tries to reinvent something that does not require reinventing.

As I have started working on library integration for screen, in light of the QML development, should I expect push backs when it comes to entity/key definition? For example, I currently have element such as library.table.sortingColumn - will I have complete freedom on how I see fit for what is my entity and key or could this create pointless PR reviews where one argues that it should be entity=library and key=table.sortingColumn, instead of entity=library.table and key=sortingColumn?

[ywwg]

To be honest, I think library integration needs its own proposal because I don't think controller scripts should be implementing library integration on their own. That is a huge task (just think of how complex our libraries are) and we do not want 50 implementations of a library widget for every controller.

Instead, Mixxx should provide a QML widget that controller scripts can plug in to their screens and customize for the available resolution as needed. The controller->screen communication would only be navigational controls: left/right/up/down/tab/enter/back etc.

As controller screens get more complex, we need to move more of the logic into mixxx, not into the controller scripts.

[daschuer]

Did you consider the Pro and cons of using QJSValue as storage container?
I think this will saves us from a conversion round trip.

I have not yet understand why it is safety issue to pass a Objects from one instance of a mapping to the another instance of the same mapping. This is all well wrapped in the QVariant/QJSValue/QMetaObject type system. Sure you can misinterpret the data, but that can already happen even with a bool.

All this becomes an issue when we go to the "global" namespace. Here we need even more than just a type definition. Every value needs also a strict businesses logic description.
We have that implicit in Mixxx with COs by the C++ domain. This is missing if it is a mapping only data. The business logic around a value might be "developing", which is a risk in all interoperable protocol definitions we have in the wild.

The lack of this business logic layer is for for my understanding the main issue why we not yet all using OSC instead of midi. They also have binary blob btw:
https://opensoundcontrol.stanford.edu/spec-1_0.html

The approach described here to start with only the instantly required datatype is good.
The variant makes user that we can extend it to any other type we need.

[daschuer]

Can you give some examples for your pro and cons arguments?

From my experience a hash single hash lookup is faster than three:

QVariant value = m_sharedValues.value(namespace + entity + key);  
QVariant value = m_sharedValues.value(namespace).value(entity).value(key);  

But that is an implantation detail that can be hidden.

The structured version allows nicely to iterated through an entity.
The first one would allow to add or remove layers dynamically.

But which requirements do we have for them?

[ywwg]

I have not yet understand why it is safety issue to pass a Objects from one instance of a mapping to the another instance of the same mapping.

it's true this is a bit hypothetical. Mostly I want to keep things simple and predictable and avoid this mechanism being abused for large data transfers it was not intended for.

All this becomes an issue when we go to the "global" namespace. Here we need even more than just a type definition. Every value needs also a strict businesses logic description.

Agree -- that's why we are not allowing global for now

The approach described here to start with only the instantly required datatype is good.

👍🏻

[JoergAtGithub]

I think we will run in a maintainabilty deadlock if mapings have deep links between it's parts. This is because maintainability of screen mappings it's propably necessary that we adjust the QML code for new Mixxx version regulary (unless we freeze the whole QML code now and forever). Therefore it's vital, that developers can test/debug QML screen mappings without having access to the hardware.
Currently we can run any screen mapping without the hardware, by rendering it's content in the controller preferences dialog. Until now all information of the QML screen mapping comes from Control Objects or a similar Mixx API. and therefore we can do everything with just using the Mixxx GUI to set these COs and we can test the screen rendered in the preferences dialog in any mode.
But there is the limitation, that COs do not cover device specific controls, like:

• context specific soft-buttons around a screen
• an encoder for track selection per screen

With a lightweight interface restricted to basic types (like boolean for button pressed), we could easily add button widgets to simulate the hardware button or a spin bow widget to simulate an encoder.

Therefore we should just forward the received information from HID/MIDI to the QML screen mapping and not interpreting it in HID/MIDI scripts. This way boolean, number and string covers all cases, and we keep 100% screen mapping testability for developers without the particular controller hardware.

[ywwg]

There is no limitation with the debugging screens that prevent them from able to show information received that is not just double or bool, the problem is in generating those signals without the hardware. But that is not a flaw in this new proposal. The problem of testability of controller mappings already exists and is not made significantly worse by this proposal. If there is a desire to make mappings testable without hardware, that is a separate concern and I don't see a big hurdle for such a solution to also support setting the data described in this proposal.

Meanwhile, now that I understand the other use-cases for shared data in this mapping, it seems clear that disallowing the complex types would greatly hamper the ability for screens to be flexible and reactive and we would be forced to remove or rewrite a bunch of the S4 mapping as described in the threads above. Remember that we only have a single controller with screen support, and it's in our interest to make it as rich and interactive as possible. I would prefer to take the chance and move forward with a proposal that lets us do that.

[acolombier]

Could we imagine accepting the current proposal as "unstable" API? This could be a way to take the current proposal to a practical test and see if the discussions (entity, type, ...) are potentially being irrational/overly cautious?

[daschuer]

How about the question: QVariant vs QJsValue?

[ywwg]

How about the question: QVariant vs QJsValue?

right, sorry. Yeah I think I will try QJsValue first! And again that would be on the c++ side, I would still like to use the SafeData types in javascript land

[acolombier]

How about the question: QVariant vs QJsValue?

I don't think using a QJsValue is a good idea, as we are working with concurrent JSEngines here. This means that you will need to duplicate the JSObject from and engine to the other. Equally, the conversion cost is negligible since it would only occur upon data change, so I would recommend sticking to QVariant

[daschuer]

I take it this way: If the Traktor S4 MK3 would have only one interface the whole issue would not exists. There might be a script with different functions for different features but sharing any data by any JS type or object would be possible.
I am not advocating doing this but let's be prepared that we can. Therefore it sound reasonable to just share a QJsValue any restrictions.

@JoergAtGithub concerns are also very valid. They are however for my understanding living on the business logic.
If we share data with Mixxx to show a GUI in the preferences window, it is no longer a private namespace.
We should be strict there. I don't know how generic we can be, but a we will learn on the go.

[ywwg]

I totally agree that, for the current S4 implementation, entity:key works well, and it’s practical for what we have now.

With the extra layer, we get easier usage for the most common cases at the expense of possibly confusing parsing for advanced use-cases we have not invented yet. If we adopt the flat model, then we guarantee parsing annoyances for the most common objects and most of the existing mapping. I would prefer to solve the problem in front of me.

Future-proofing is best-effort based on available knowledge, not an exact science. We have a great foundation of experience with your hard work on the S4 mapping, so it's great to have something concrete to look at and base the design on! And that design points toward a double-layer of keys (with namespace as layer 3 on the outside).

As for the ordering of entity and key, controller mappings are logically contructed deck-first: a controller object contains deck objects contains a pads object. So I prefer to keep that structure mirrored in the data representation. Again: consistency.

tbh I have said all I can say on this topic, if you would like to block this proposal on the question of double vs single layer, let's put it to a vote.

you will need to duplicate the JSObject from and engine to the other.

There are typos here, but I think you are worried about connections from the shared data QJSValue (note: not QJSObject) to the original javascript. It does look like sometimes this is the case (https://doc.qt.io/qt-6/qjsvalue.html#toNumber) but I think for simple types, perhaps not? @daschuer are you worried about this? I do think we want to keep the controllermanager isolated from the JS engine.

[ywwg]

All of the docs have the caveat "if this QJSValue is an object" so I think by excluding objects we eliminate the danger of side-effects on the js engine.

[JoergAtGithub]

If we share data with Mixxx to show a GUI in the preferences window, it is no longer a private namespace.

That was not my point, the namepace is not an issue, as it's shown on the controler specific tab only. The point is that of now, the screen is rendered to the preference screen and you can currently use anything that this screen mapping supports, because everything depends on Control Objects (and similar Mixxx APIs). For example you could click on the PLAY button in the Mixxx main window and the waverform on the controller screen starts running.
My point was that it must remain possible to run the screen mapping without the hardware and the HID mapping. And that it must always be possible to access any function of the screen mapping without hardware.

[ywwg]

I think that's all the notes addressed

[acolombier]

Looking good now! Thank you for reviving this.

[ywwg]

@daschuer would you be able to do one more review pass before merge?

[JoergAtGithub]

Here are the findings of the requested review, primarily editorial changes and some inconsistencies between chapters.

[JoergAtGithub]

The namespace is already part of some mapping XML files now, like:
<controller id="Traktor" namespace="S4MK3">

[JoergAtGithub]

Suggested change

	`engine.getSharedData(entity: Entity, key: string): SafeData?`
	`engine.getSharedValue(entity: Entity|string, key: string): SafeData?`

The term Value would better fit to the purpose (as it makes clear this is not intended to store large BLOBs) and corresponds to the terminology we already use elsewhere in the controller engine API.
Type string is missing here to correspond to the textual description above

[JoergAtGithub]

I added this suggestion only for this one example, but it applies to the other API examples too.

[ywwg]

I don't actually know typescript well and wasn't paying attention, does this mean "Entity or string" or does it mean "Entity as string"? I think we want to restrict entity names to the limited set right now. If someone wants to add one, they can add it in their controller PR.

[ywwg]

I've updated the namespace and entity sections based on what we discussed at the monthly call.

[ywwg]

leaving unsquashed since we'll squash on merge

[acolombier]

All looking good still - just added some context from yesterday's meeting.

[acolombier]

No change required, but just to highlight what was discussed in the meeting. The mapping developer will be able to define the namespace in two different way using the mapping manifest (XML):

• A static namespace, common to all mapping instance, e.g CDJ-2000. In this case, all instances of mappings for any CDJ devices plugged will share the same namespace. This may look like something as

  <controller namespace="CDJ-2000">
    ..
  </controller>

• A device-bound namespace - whether we use we the USB serial number (unlikely due to common clash existing across devices) or device handle (Bus X.Port Y or whatver form this takes a different OSes) or some other means is implementation details and not publicly exposed the namespace name to the mapping - in this case, we ensure proper the data is only shared across any instance of the mapping that could relate to this device, and failure to do so can be considered a bug, which we will intent to fix.
  As nice to have or future iteration, we will allow a user to change. This may look like something as

  <controller deviceBoundNamespace="true">
    ..
  </controller>

As a nice to have, or future iteration, we may allow the user to explicitly override the namespace, for example with a small dropdown/combobox/inputbox or whetever UX we decide on the controller setting, unrelated to controller settings.

[acolombier]

leaving unsquashed since we'll squash on merge

Oops, I merge with the wrong mode - I took the freedom to squash manually and force-push the the new commit here