-
Notifications
You must be signed in to change notification settings - Fork 10
Controller Shared Data: Initial Proposal #17
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
Changes from all commits
da9f1ff
54c539e
8bce9f9
207bc35
ccac383
c44cf03
bcf5098
a7bc66f
4b6ebfa
865ef6f
e92fbee
64f00a2
299ebb2
d3923e9
82b3559
4f18533
c87b346
c74b978
7fcb53f
9505f69
9c71d50
38d7025
d9fee84
222f7ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,248 @@ | ||
| # Controller Shared Data | ||
|
|
||
| * **Owners:** | ||
| * `@ywwg` | ||
| * `@acolombier` | ||
|
|
||
| * **Implementation Status:** `Partially implemented` | ||
|
|
||
| * **Related Issues and PRs:** | ||
| * [Ability for controller to share data at | ||
| runtime](https://github.com/mixxxdj/mixxx/pull/12199) | ||
|
|
||
| > TL;DR: Allow controllers mappings to set and retrieve variables of different | ||
| > data types in order to exchange them between the controller code and the | ||
| > engine. Think: ControlObjects of arbitrary type that controllers can declare. | ||
|
|
||
| ## Why | ||
|
|
||
| There are multiple scenarios where controller mapping scripts need to share and | ||
| access data outside the container of their own controller script engine. This | ||
| includes situations where some controllers expose more than one USB interface | ||
| that need to communicate with each other, or when a DJ connects multiple | ||
| instances of the same hardware to Mixxx. | ||
|
|
||
| ### Pitfalls of the current solution | ||
|
|
||
| ControlObjects are the normal way we share data, but: | ||
|
|
||
| * Controllers can't declare control objects. | ||
| * Putting controller-specific data inside Mixxx itself would be bad and lead to | ||
| bloat. | ||
| * Controllers need to share more types of data than just double values. | ||
|
|
||
| ## Goals | ||
|
|
||
| Goals and use cases for the solution as proposed in [How](#how): | ||
|
|
||
| * Namespacing: Allow different controllers to declare same-named data objects | ||
| without risk of collisions | ||
| * Support controllers with screens that require communication from HID to | ||
| separate Bulk USB devices (Traktor S4 MK3). | ||
| * Build a data model foundation for users with multiple instances of the same | ||
| controller (CDJ-2000). | ||
| * Ensure that the API could support the features we may wish to add in the | ||
| future, such as global namespace, without breaking controller mappings that | ||
| use the API defined here. | ||
|
|
||
| ### Audience | ||
|
|
||
| Users of modern controllers or multiple controllers will appreciate this work. | ||
| Specifically, this work is required to fully support the Traktor S4 MK3, which | ||
| has separate USB interfaces for the controller and the two screens (two total USB | ||
| interfaces). | ||
|
|
||
| ## Non-Goals | ||
|
|
||
| * We do not intend to fully support the multi-device scenario yet, that needs | ||
| further design to associate specific devices with specific controller | ||
| configurations. | ||
| * We do not intend to immediately support "global" objects accessible across | ||
| namespaces, like "universal shift". | ||
|
|
||
| ## "Universal Shift" | ||
|
|
||
| "Universal Shift" refers to the idea that a shift button pressed on one | ||
| controller can be detected by any and all other controllers. This may be a | ||
| useful use-case but creates a lot of difficulties, so for now Universal Shift is | ||
| out of scope for this first implementation. | ||
|
|
||
| ## How | ||
|
|
||
| ### Data Object | ||
|
|
||
| We will create a central object inside Mixxx that contains a triple-keyed map: | ||
|
|
||
| * Namespace (string) | ||
| * Entity (string) | ||
| * Key (string) | ||
| * Value ("SafeValue") | ||
|
|
||
| #### Namespace | ||
|
|
||
| `Namespace` is a string that is unique to each controller **mapping | ||
| definition**. All hardware mapping configurations must specify distinct | ||
| namespaces. | ||
|
|
||
| Multiple device support is still out of scope for this proposal, but we | ||
| anticipate that this design can expand to support that use case. For example, | ||
| the namespace MAY have a suffix appended to distinguish distinct devices based | ||
| on an automatically-detected unique device identifier, e.g. two CDJ-2000's could | ||
| have namespaces like `CDJ_2000-ABCDEF` and `CDJ_2000-FEDBCA`. This suffix would | ||
| be applied in C++ code outside of the awareness of the controller mapping. | ||
|
|
||
| If a unique serial number cannot be determined at runtime, a special controller | ||
| preference (defined in Mixxx, not controller mappings) could be used to map | ||
| which controller is associated with which device through a new API, and this | ||
| value will be passed to controller mappings. | ||
|
Comment on lines
+87
to
+97
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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):
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. |
||
|
|
||
| #### Entity | ||
|
|
||
| `Entity` is a logical value defined by the controller mapping definition. It can | ||
| be like a Mixxx-style group ("`[Channel1]`") but it can be any of the | ||
| preselected names listed below. Many controllers will want to define something | ||
| like `deck1` to refer to a device that can itself be assigned to multiple mixxx | ||
| channels. | ||
|
|
||
| The controller mapping decides how these entities behave and Mixxx does not | ||
| enforcement of them. To reiterate: even if an "entity" "looks like" a Mixxx | ||
| group, it is not. | ||
|
ywwg marked this conversation as resolved.
|
||
|
|
||
| Here is the proposed initial description for entity definition, which is | ||
| anticipated be extended over time. | ||
|
|
||
| ```typescript | ||
| declare Entities { | ||
| type Mixer = 'mixer'; | ||
| type Main = 'main'; | ||
| type Library = 'library'; | ||
| type Decks = 'deck1' | 'deck2'; | ||
| type Channels = 'channel1' | 'channel2' | 'channel3' | 'channel4'; | ||
| type Controller = 'controller'; | ||
| } | ||
| type Entity = Entities.Mixer | Entities.Main | Entities.Library | Entities.Decks | Entities.Channels | Entities.Controller; | ||
| ``` | ||
|
|
||
| Numbered entity names could also be validated with a regular expression such as | ||
| `deck[0-9]+`. | ||
|
|
||
| 'Entity' values are enforced by Typescript library code before function calls | ||
| are handed off to C++. | ||
|
|
||
| #### Key | ||
|
|
||
| `Key` is a logical value defined by the controller mapping definition. It could | ||
| refer to a button, light, knob, or abstract name. | ||
|
|
||
| The controller mapping decides how these keys behave and Mixxx does no | ||
| enforcement of them. Similar to "entity", keys bear no relation to equivalent | ||
| Mixxx keys. | ||
|
|
||
| #### Value | ||
|
|
||
| In the engine, the `value` is stored as a QVariant, however we want to only | ||
| support a limited set of types in Javascript / Typescript. For the first | ||
| implementation, we will support bool, number, and string, and Arrays of those: | ||
|
|
||
| ```typescript | ||
| type SafePrimitive = string | number | boolean | null; | ||
|
|
||
| type SafeValue = | ||
| | SafePrimitive | ||
| | SafePrimitive[]; | ||
| ``` | ||
|
|
||
| The list of allowable types can be expanded as needed, but we want to be sure | ||
| that the shared data system does not become a "bag of bytes" message bus for | ||
| large pieces of data like bitmaps or code, nor should it be used to circumvent | ||
| intentional limitations or gaps in the overall javascript framework. | ||
|
|
||
| #### Example | ||
|
|
||
| The shift button on the left side of a Traktor S4MK3 would be stored this way: | ||
|
|
||
| pseudocode -- not final naming: | ||
|
|
||
| `m_shared_value["S4MK3"]["deck1"]["shift"] = true` | ||
|
|
||
| ### API | ||
|
ywwg marked this conversation as resolved.
|
||
|
|
||
| The shared data API should be roughly the same across Controllers, QML, and C++. | ||
| The primary difference is that C++ will have access to the namespace value at | ||
| all times, whereas controllers and QML will have that value elided. | ||
|
|
||
| The controller javascript has access to the shared value object through three | ||
| functions: | ||
|
|
||
| #### Get | ||
|
|
||
| excuse the pseudo-js: | ||
|
|
||
| `engine.getSharedValue(entity: Entity, key: string): SafeData?` | ||
|
|
||
| `namespace` is set automatically by the engine code, so controllers can't get | ||
| that wrong. | ||
|
|
||
| This function returns error if the value is not found. | ||
|
|
||
| #### Set | ||
|
|
||
| `engine.setSharedValue(entity: Entity, key: string, value: SafeData): void` | ||
|
|
||
| `namespace` is set automatically by the engine code, so controllers can't get | ||
| that wrong. | ||
|
|
||
| Calling "set" triggers "updated" signals to all subscribers across the engine, | ||
| QML (e.g. controller screen code), and controllers. | ||
|
|
||
| Controllers do not get notified about updates they initiated themselves, to | ||
| prevent circular signal loops. | ||
|
|
||
| #### Updated | ||
|
|
||
| Controllers can subscribe to notifications about data updates via a method | ||
| similar to how they subscribe to engine Control Object updates: | ||
|
|
||
| (not final naming) | ||
|
|
||
| `function makeSharedValueConnection(entity: Entity, name: string, callback: CoCallback): ScriptConnection | undefined;` | ||
|
|
||
| In this first implementation, controllers can only subscribe to updates for | ||
| their own namespace. | ||
|
|
||
| ### Possible future directions | ||
|
|
||
| The following are possible future extensions to this proposal that are currently | ||
| out of scope and will not be implemented in the first version, but we want to | ||
| make sure to leave room in case we add them in the future: | ||
|
|
||
| #### Cross-device communication / subscription | ||
|
|
||
| There is a possibility controller authors may want access to signals sent from | ||
| other controllers, for instance a "universal shift" button. For this purpose we | ||
| may choose to allow controller to "subscribe" to updates from other namespaces, | ||
| or all namespaces, in a read-only fashion. In this scenario, controllers would | ||
| not have the ability to write updates to namespaces outside their own. This may | ||
| be brittle because controllers would need to know about all possible valid | ||
| namespaces, so this feature would need more care to make it maintainable. | ||
|
|
||
| #### "Global" namespace | ||
|
|
||
| Another possibility is that we may want a "global" namespaces that all | ||
| controllers can read and write to. This would be another way to support a | ||
| "universal shift" button. This would have to be carefully managed to prevent | ||
| collisions between controller configs. One way to do this would be to "bless" | ||
| specific entities and keys for the global namespace, and controller authors | ||
| would have to add their requested global entity/key to Mixxx. | ||
|
|
||
| ## Alternatives | ||
|
|
||
| The original implementation did not have entities and keys and instead had a | ||
| single namespaced data blob that controllers had to manage themselves. This | ||
| approach requires a lot more work on the part of the controller author to merge | ||
| and manage the data object. | ||
|
|
||
| ## Action Plan | ||
|
|
||
| 1. Build on the existing PR to implement the desired API | ||
| 2. Rewrite the Traktor S4 MK3 mapping to support the new API. | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of adding another dimension here, as opposite to stricly rely on the key, such that
(["deck1", "button")would becomedeck1.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?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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...).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give some examples for your pro and cons arguments?
From my experience a hash single hash lookup is faster than three:
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea was to use a single hash with three key, much like your first line, but not sure.
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.slot1could still be interpreted in a tree structure thanks to the., without constrain on the number of layer, as opposite the entity/key design)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that looking at the scale of a
namespace, the hashing performance is probably anecdotal.I totally agree that, for the current S4 implementation,
entity:keyworks 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 likepadsmode:deck.group? Would it beentity=padsmode, key=deck.group, orentity=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 flatkeywould be easier.Now, there is a risk than once mapping uses
padsmode.deck.groupand another onedeck.padsmode.key, but at least, this would be one single symbol (either a variable or a literal)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 myentityandkeyor could this create pointless PR reviews where one argues that it should be entity=libraryand key=table.sortingColumn, instead of entity=library.tableand key=sortingColumn?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.