Skip to content
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

Add support for SD+ #44

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add support for SD+ #44

wants to merge 3 commits into from

Conversation

rdohms
Copy link

@rdohms rdohms commented Feb 12, 2023

With the release of SD+ you now have control over dials and touchscreen on top of the usual buttons. Support adds a few new events received and sent and new configuration objects like Encoder and Layout.

PR Checklist

PR Structure

  • This PR has reasonably narrow scope (if not, break it down into smaller
    PRs).
  • This PR avoids mixing refactoring changes with feature changes (split into
    two PRs otherwise).
  • This PR's title starts is concise and descriptive.

Thoroughness

  • This PR adds tests for the most critical parts of the new functionality or
    fixes.
  • I've updated any docs, .md files, etc… affected by this change.

What

Adds support for the new controls available in the SD+

Why

SD+ now has dials and a touch screen to control.

Known limitations

Testing this out in a new plugin before I come back to check if we can improve the experience. Putting this up as a draft so ppl can test it on their own and give any early feedback.

@rdohms
Copy link
Author

rdohms commented Mar 13, 2023

To Do Fixes

  • Improve Event signatures with more properties
  • The layout approach needs a re-think

@rdohms
Copy link
Author

rdohms commented Mar 21, 2023

Ok, the events look good now, and I was able to build a plugin with this.
The Layout code is not really needed at this point, the constants are useful, but it appears custom layouts need to be used via actual files which is not great, but I think we leave them in case someone wants to build the layout programmatically and have webpack save it as a file for usage, which should be doable.

The test breakage seems unrelated, how can I help there @fnando? Would love to get this into a new release so I can release the plugin.

@rdohms rdohms marked this pull request as ready for review March 26, 2023 10:57
@rdohms rdohms requested a review from fnando as a code owner March 26, 2023 10:57
@rdohms
Copy link
Author

rdohms commented Mar 27, 2023

Actually, I found something we may want to improve before releasing.
In the action, you can now include Encoder as a controller. However, that means It just adds to the controller array, so your action will have both KeyPad and Encoder.
Ideally, there should be a way to set action.keypad to false so it omits that for Actions that are purely designed for dials.
I think a action.keypad that defaults to true and can be set to false is the best approach without breaking back compact. WDYT?

rdohms and others added 3 commits April 8, 2023 13:38
With the release of SD+ you now have control over dials and touchscreen
on top of the usual buttons. Support adds a few new events received and
sent and new configuration objects like Encoder and Layout.
In case of new SD+ plugins, they may be allowed on either a KeyPad or a
Dial (via Encoder). Adding this property `keyPad` allows us to disable
the options of installing an Action to the KeyPad, forcing it to be
Encoder (Dial) only.
@rdohms
Copy link
Author

rdohms commented Apr 8, 2023

Added the handling for Encoder or KeyPad and tested it, it works great, with no BC.
Fixed the recommendations you made on explicit strings and where I messes up numbers and strings.

* @param {DialPressEvent} event The event data.
* @return {void}
*/
handleDialPress(event: DialPressEvent): void {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dialPress is now deprecated in favour of dialDown and dialUp

@mmanciop
Copy link

I am trying to get this PR to run, and there is some bit rot in the dependencies. For now, I am using these Yarn resolutions added to the package.json to run tests:

  "resolutions": {
    "string-width": "4.2.3",
    "glob": "9.3.5",
    "strip-ansi": "6.0.1",
    "wrap-ansi": "7.0.0"
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants