New Controller Engine #7
Conversation
|
Thank you for you proposal. This is not an alternative to #6, because it has a different abstraction level. I have filed #6 to compare all ideas with more detailed plans like here. This way we can always cross reference that we don't miss a point. Are all requirements possible to implement with this proposal? |
|
I think so yes. I mean I'm not waterfall-design expert, so a little bit of this needs experimentation, but I think the architecture is at least flexible enough for the future. The only major risk is that this flexibility will make other features more difficult to implement. For example, I don't have a clear idea on how well the automatic pairing of sources and sinks will work. I briefly touched on that in the "Current Unknowns" section of the proposal. |
| containing at least a manifest file named `manifest.xxx` (where `.xxx` is the | ||
| language-specific file extension (eg. `.xml`, `.yml`, `.json`, etc)). | ||
| Additionally, `res/controllers/lib` contains shared modules that can be used by | ||
| multiple mappings (such as componentsJS). |
There was a problem hiding this comment.
We can even allow to share mappings via an unpacked zip file with a fancy extension that Mixxx is able to unpack.
I just remember discussions that mapping writers want to work only in a single file. So we need to be clear about our goals here.
There was a problem hiding this comment.
I personally find it a little silly to zip files and then use a custom extension (looks like a bad attempt at hiding something) but from a technical perspective, I don't see anything wrong with that. Its just an extra step required while exporting. Importing is probably suboptimal if we would unpack the file during each scan to read the manifest.
There was a problem hiding this comment.
I personally find it a little silly to zip files and then use a custom extension (looks like a bad attempt at hiding something)
It's less about hiding something and more about saying "this is a mixxx-mapping" so that we could add an "Import Mapping' button to the controller preferences that allows picking a file from the downloads directory and automatically copies it to the right location without listing every single non-mixxx-related zipfile in the directory.
Importing is probably suboptimal if we would unpack the file during each scan to read the manifest.
I suppose the performance impact is neglible. As an alternative, mixxx could provide an "export button" that zips the file w/o the manifest and includes the manifest text in the zip comment if we're worried about that.
There was a problem hiding this comment.
yeah, all pretty fair suggestions. We'll just need need a good library for it I guess because Qt doesn't have any facilities afaict.
There was a problem hiding this comment.
It's less about hiding something and more about saying "this is a mixxx-mapping"
I guess we could do something similar than for NI stem format and add a prefix to the extension (e.g .mixxx.zip or .mixxx.tar.gz). This would also us to support a wide range of compression format.
Importing is probably suboptimal if we would unpack the file during each scan to read the manifest.
Probably getting too much into implementation details, but we could inspire from tools that uses a similar mechanism. Thinking of tools like Terraform or Helm that also have many "third party" extension compressed in an archive, you can expand them into a "cache" folder on import and maintain a checksum comparison on the source archive. As long as that checksum doesn't change, you can safely use the manifest file from the filesystem. This also greatly simplify modding as users can change the expanded module on the fly, without having to use a different mechanism (some kind of "draft" folder) or need to repack an archive.
TL;DR; I don't think this is a problem.
There was a problem hiding this comment.
This would also us to support a wide range of compression format.
This would really be a non-goal for me. Then we need to support all these compression formats forever, and I think there is no good reason for it. Just agree on a single format (or rather two: packed for easy exchange and unpacked for easy development).
There was a problem hiding this comment.
Many file formats (like OpenDocument for Office application) use JAR files as container (just with a different file extension). The JAR file container defines the zipping and the manifest file format. And also handle topics like fast reading the manifest without unziping the whole files.
But I don't think that JAR files are good for the developer experience of a mapping developer. I suggest to use just sub-directories per mapping.
| * Sources and Sinks | ||
| * Execution Engines | ||
| * Used Mixxx APIs | ||
| * Capabilities |
There was a problem hiding this comment.
I am afraid this flexibility will introduce lot of boilerplate for no reason. I would prefer to straight forward code a new engine without adding facilities for a not yet known future engine. YAGNI
That does not mean to not allow such future engine, it is always a balance.
There was a problem hiding this comment.
Do you mean boilerplate in the C++ domain or just in the manifest? For the manifest I could imagine adding some shortcuts for the common usecase sure, but I'd like to keep the decoupled nature on the C++ side. Otherwise we'll be right back where we started with the current engine.
There was a problem hiding this comment.
This is more a general concern. This flexibility and abstraction to allow different engines, comes with the cost that things are harder to implement and cannot be optimized because we have only the common ground of all engines.
So I would rather focus on solving the issues with the current engine and not to be full generic and future proof.
| interacting with an execution engine via this section. This essentially works | ||
| like the current mapping editor, but removes the | ||
| `<key>path.to.some.JS.input.handler</key>` feature as it is not compatible with | ||
| the new architecture. This method is favoured over code generation as that is |
There was a problem hiding this comment.
We have failed to implement that in other protocols than midi.
To allow it for BULK and HID, we need a "driver" layer that converts the proprietary telegrams into something usable for a reactive implementation. This will probably end up in two script stages. 1. The driver 2. The mapping.
Did you consider this?
There was a problem hiding this comment.
Not yet, The dispatcher. I know @acolombier made some experiments regarding that. I'm sure a dispatcher functionality could work for protocols other than midi (at least as long the data format is simple enough) but I consider them out-of-scope for this PR. It wouldn't be super hard to add on later though.
There was a problem hiding this comment.
It wouldn't be super hard to add on later though.
Than lets at least consider it right form the start. It would be an immediate user benefit to have all back-ends GUI learnable.
There was a problem hiding this comment.
I have build a working PoC which allows to learn action by implementing a naive diff on HID report. Certain venodr (like NI) also appears to be providing a fairly complete HID descriptor. This means that we could be able to implement a HID layer for it. I don't think it would be possible with BULK tho, but I believe that controller using BULK for input are becoming rare nowadays.
There was a problem hiding this comment.
For my understanding it is only a difference how generic such a driver will be. I can Imagine to build a driver for a bulk controller with a weird protocol without any self documentation.
At this point, I just want to make sure our data flow allows it.
There was a problem hiding this comment.
I'm fairly confident it will if we adapt the sources and sinks model...
| all the required files in the root of the mapping during exporting. That tree is | ||
| then overlaid over the built-in libraries. This is currently only possible | ||
| within a `QQmlEngine` using `QQmlEngine::addUrlInterceptor` and | ||
| `QQmlEngine::addImportPath`. |
There was a problem hiding this comment.
This sounds scary to be honest. Remember that we need to catch users with school level programming skills.
Every high level implementation may scare them away.
There was a problem hiding this comment.
Can you elaborate on how that's scary?
All I'm saying is that we would export the libraries loaded from outside the mapping folder and copy those into mapping. Then when we load the mapping we prefer the versions bundled with the mapping over those built into mixxx. This avoids issues where users would complain when the mapping they download from the forum suddenly breaks / works differently when they run it on a different mixxx version. That allows us to have less strict stability requirements for our libraries.
There was a problem hiding this comment.
It probably sound scary for me because I did not understand it.
My general concern is that we should keep the required programming knowledge at a minimum. With a stable API and a complete documentation. The idea presented here sounds like a receipt for the opposite, but I could be wrong.
There was a problem hiding this comment.
My general concern is that we should keep the required programming knowledge at a minimum.
I agree. All I'm proposing here is how we can ensure we bundled and load the correct shared (across mappings; eg common-controller-scripts.js) dependencies when the mappings are being shared on the forum. This is essentially the same we already do when overlaying .mixxx/controllers/ over res/controllers/.
|
Thanks for putting this together @Swiftb0y ! I will have a detailed read soon. |
| interacting with an execution engine via this section. This essentially works | ||
| like the current mapping editor, but removes the | ||
| `<key>path.to.some.JS.input.handler</key>` feature as it is not compatible with | ||
| the new architecture. This method is favoured over code generation as that is |
There was a problem hiding this comment.
I have build a working PoC which allows to learn action by implementing a naive diff on HID report. Certain venodr (like NI) also appears to be providing a fairly complete HID descriptor. This means that we could be able to implement a HID layer for it. I don't think it would be possible with BULK tho, but I believe that controller using BULK for input are becoming rare nowadays.
| like the current mapping editor, but removes the | ||
| `<key>path.to.some.JS.input.handler</key>` feature as it is not compatible with | ||
| the new architecture. This method is favoured over code generation as that is | ||
| not always possible, nor would it likely produce a good result. Moreover, this |
There was a problem hiding this comment.
I would disagree with this statement. Yes, it is correct for JavaScript, especially as we have very different styles of JS across on the different mapping.
However, code generate in QML should be possible and provide fairly good result. Since JS is QML, there would also be no value loss in using QML. This would mean that it would be possible to have fully hybrid mapping, where non-technical user can easily tweak the behaviour of button, share the same mapping and have technical user implementing more complicated logic in JS, on the same source, without having some kind of "override" logic hidden and abstracted in the manifest file.
I guess we could still consider this approach for JS mapping so we provide feature parity, but could "default" to QML when creating mapping on the wizard.
I would also add that, as per the PoC I did on point-and-click, using QML could help us to optionally capture valuable metadata to help with the representation of the controller for interactivity purpose, customisation and educational purpose.
Kooha-2024-05-28-21-24-49.mp4
There was a problem hiding this comment.
The simple standard mappings must be stored in a semantic language, which allows the mapping wizard to read the already mapped controls. This must work for MIDI and HID.
There was a problem hiding this comment.
I have two issues with code generation:
- In the case of QML, there is no library that can manipulate / build up a QML AST and then serialize it into a mapping. Any other solution involving formatting text into a buffer is too brittle IMO.
- Since writing Qt APIs in C++ involves lots of boilerplate, I would prefer to write as much code as possible in QML (including a hypothetical ComponentsQML library). This hypothetical code generation tool would ideally generate ComponentsQML code. The result is C++ code depending on QML code. This is in turn, is again too brittle IMO.
without having some kind of "override" logic hidden and abstracted in the manifest file.
I didn't really intend there to be override logic. At least not directly. Each protocol can opt into using a dispatcher module (instead of just getting all its traffic dumped to some handler directly). Then the manifest can specify a CO to be controlled and the execution engine can specify some code to executed based on receiving some message. If both the manifest and the execution specify something for the same message, the action declared in the manifest takes precedence.
Does that make sense?
There was a problem hiding this comment.
A code generator is not what is needed, we need an editor! Which allows to read a mapping into the GUI and allows to re-map the controls.
There was a problem hiding this comment.
The simple standard mappings must be stored in a semantic language
Agreed, this is why, due to its declarative nature, QML could be a good pick.
This must work for MIDI and HID.
Not sure why this wouldn't? This proposal suggestd a new engine agnostic to the IO backend, but also stands as an independent implementation, that doesn't have to live with the legacy engine, but rather be an alternative. At least I believe that was the the ambition when we discussed about it with @Swiftb0y
A code generator is not what is needed, we need an editor! Which allows to read a mapping into the GUI and allows to re-map the controls.
That's exactly what Qt Creator does. And all the relevant source is under compatible license AFAIK. QML is a declarative language, and custom QML component can allow to keep mappable property in the "code" using well constrained property
In the case of QML, there is no library that can manipulate / build up a QML AST and then serialize it into a mapping. Any other solution involving formatting text into a buffer is too brittle IMO.
qtdeclarative and qt-creator(particularly src/qmldom) can provide that. They are also maintain by Qt itself, so they provide premium support of the QML standard going forward.
writing Qt APIs in C++ involves lots of boilerplate
I think this statement is not always true. QML boilerplate can be extremely simple if we design the engine in a sensible manner. My previous attempt as part of #11407 shouldn't be taken as reference as it was attempting to align as much as possible with the legacy JS engine it would live in. Now assuming we make separate agnostic engine (or even QML only), this could be greatly simplified, and potentially contain even less boilerplate than bare JS
I didn't really intend there to be override logic. At least not directly. Each protocol can opt into using a dispatcher module (instead of just getting all its traffic dumped to some handler directly). Then the manifest can specify a CO to be controlled and the execution engine can specify some code to executed based on receiving some message. If both the manifest and the execution specify something for the same message, the action declared in the manifest takes precedence.
It does make sense, and was what I had in mind already. But this is what I meant by the override logic: instead of having a single source of truth, which follow the same API, you end up with two APIs which aren't compatible. This means that someone with a limited knowledge cannot use the point-and-click to establish a basic behaviour and then override certain hooks with simple logic, especially when trying to implement new controller, not yet supported.
| containing at least a manifest file named `manifest.xxx` (where `.xxx` is the | ||
| language-specific file extension (eg. `.xml`, `.yml`, `.json`, etc)). | ||
| Additionally, `res/controllers/lib` contains shared modules that can be used by | ||
| multiple mappings (such as componentsJS). |
There was a problem hiding this comment.
It's less about hiding something and more about saying "this is a mixxx-mapping"
I guess we could do something similar than for NI stem format and add a prefix to the extension (e.g .mixxx.zip or .mixxx.tar.gz). This would also us to support a wide range of compression format.
Importing is probably suboptimal if we would unpack the file during each scan to read the manifest.
Probably getting too much into implementation details, but we could inspire from tools that uses a similar mechanism. Thinking of tools like Terraform or Helm that also have many "third party" extension compressed in an archive, you can expand them into a "cache" folder on import and maintain a checksum comparison on the source archive. As long as that checksum doesn't change, you can safely use the manifest file from the filesystem. This also greatly simplify modding as users can change the expanded module on the fly, without having to use a different mechanism (some kind of "draft" folder) or need to repack an archive.
TL;DR; I don't think this is a problem.
Yes, that is WIP, I just need to find some time to finish it. |
|
Thanks for your feedback and comments in the meantime @acolombier |
| Sources and Sinks are the endpoints of the controller. They are defined in the | ||
| manifest and are used to define the IO protocol of the controller. They are | ||
| characterized by their protocol (eg. MIDI, HID, Bulk, etc), their direction (eg. | ||
| input, output, bidirectional) and optionally explicitly their underlying |
There was a problem hiding this comment.
Why should I specify the direction for HID or BULK report? This information can be read from the device.
There was a problem hiding this comment.
Can you elaborate?
These sections should allow the mapping to declare things like "I need an HID endpoint and I intend to read from it". Then if no hardware with that capability is found, the mapping can not be loaded in the first place.
There was a problem hiding this comment.
Every USB device has "Endpoint Descriptors" ( https://beyondlogic.org/usbnutshell/usb5.shtml ), which contain this information per enpoint. If I know the supported device type, why should I store a copy of this information in the mapping files?
There was a problem hiding this comment.
good point. Primarily so the mapping can declare stable identifiers to address them. What do you propose instead?
There was a problem hiding this comment.
I propose to assign the device type to mapping by "Manufacuter ID" and "Product ID" and read out all information of the device, which we can gather via userspace APIs. Only what is not accessible on all supported systems, shall be stored in the mapping files.
There was a problem hiding this comment.
I think there is a risk of doing a 1-to-1 relationship here. Having the IO driver respecting the USB descriptor is definitely a must have, but it would make sense to also explicately define the direction the mapping is interested with. This would help us planning for vendor which may be using not so compliant descriptor (BULK being probably the most at risk here, since it already involve a custom API made by the vendor), meaning that the port could be used for more than just receiving/sending mapping relevant data.
There was a problem hiding this comment.
I'm still not convinced how this direction property should be clearly seperated, e.g.:
- A BULK mapping might need to send a memory address before reading a value. While it is only done for reading, it technically would send a BULK message with an address.
- HID allows to read the value of the OutputReport back from the device (e.g. for seamless initialization at hot-plugging) Does this make any OutputReport to a bidirectional Source/Sink?
For what is this information needed for?
Why do we need to enforce this by the top level architecture. I fear that this will restrict us in the future, as we will always have to force all USB protocols into this arbitrary structure.
There was a problem hiding this comment.
I think your concerns are valid. So the consequence is that we only apply the source/sink abstraction to midi. So no abstraction over these protocols. That may be fine, but then we would have to reimplement the PnC logic for each protocol. Do you have some other abstraction in mind?
There was a problem hiding this comment.
We need an entity to assign to the execution engine here, but this must be an abstract definition to be future proof. It should cover cases like:
- An USB interface for BULK mode
- MIDI1 and MIDI2 devices
- An USB interface, that appears as two MIDI "devices" with https://github.com/mixxxdj/mixxx/blob/a65bc0bf3ac13114b2d7a8a2474394768a321c27/src/controllers/midi/portmidienumerator.cpp#L76C1-L93C1
- An USB HID interface with two top-level-collections, which appears under macOS and Linux as one HIDAPI device
- An USB HID interface with two top-level-collections, which appears under Windows as two HIDAPI devices
- A MIDI device that is accessible over two physical interfaces (like the DDJ-FLX4 with MIDI over BluetoothLE and USB)
There was a problem hiding this comment.
Thanks for the comprehensive collection, but it doesn't answer my question. What do you imagine the common API between these to look like? We need some abstraction unless we want to reimplement common features such as the PnC editor for each protocol.
Co-authored-by: JoergAtGithub <64457745+JoergAtGithub@users.noreply.github.com>
|
I don't really have anything intelligent to say about this proposition but I'm eager to start working on it ASAP. |
|
I think the first step would be to come up with an suitable interface that represents an abstract controller manifest, as well as a modular info section. That would be the PoC that proves that the design works in a language-agnostic fashion and that sections can be added or removed from it without too much trouble... |
|
Like an IDL? Could protobuf be used to do that? Or we need an more abstract language? Edit: I thought Swagger was retricted to describing and documenting HTTP API, but I'm not so sure anymore. I've only used it for HTTP APIs myself. |
|
No, code architecture design flexible enough to accommodate our requirements, no external tools or languages. Just C++ mockup code essentially. |
|
Something like Swagger could be useful to specify and document the format in a language-agnostic way though. Whereas C++ is hard to read. |
|
I think we're talking past each other. The issue is not about how the API will look like exactly, its about how to use the features offered by C++ efficiently to build an expressive C++ API. Last time I checked, swagger was for specifying Web (usually rest) APIs, which is completely unrelated to this... |
|
Oh sorry, I though you wanted the first step to be specifying the XML/YAML/JSON/TOML controller metadata format. My bad. |
|
not really. Its the abstraction layer above that: how do we represent the manifest without tying ourselves to a specific format? My best guess right now is a set of pure virtual classes that define the different sections. Specific format implementations then derive from those classes (effectively interfaces). I'm not sure if that design is suitable yet though. C++ doesn't quite give me the facilities I'd like it to... |
|
I did some research but I'm surprised there seems to be no generalist ser/deser library like Jackson for Java… Edit: how about https://github.com/getml/reflect-cpp? |
My Alternative to #6
CCing @acolombier because we have brainstormed some of these requirements in a call some time ago. I have since rewritten the notes into the proposal format here, does it still match what we brainstormed together?
Note that this does not actually go that deep into detail on how a QML-based engine should look like, rather it tries to lay the foundations for a more modular design we can more easily iterate on.