Skip to content

Added Assignment updates/ variables #93

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.

Sign up for GitHub

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

Open
kokofi3 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Added Assignment updates/ variables #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/midi/parse/channel-parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ export interface MixerMessageEvents {
* level specified by `vc`/`vf`.
*/
pan_level: [nrpn: NRPN<'panBalance'>, vc: number, vf: number]

/**
* The assignment status of a source in a sink (assign NRPN) with the data
* bytes `vc`/`vf`.
*/
assign: [nrpn: NRPN<'assign'>, vc: number, vf: number]
}

/**
Expand Down Expand Up @@ -149,6 +155,10 @@ export class ChannelParser extends EventEmitter<MixerMessageEvents> {
// Pan Level
else if (0x50 <= msb && msb <= 0x5f) {
this.emit('pan_level', makeNRPN(msb, lsb), vc, vf)
}
// Assign NRPNs
else if (0x60 <= msb && msb <= 0x6f) {

@jswalden jswalden Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Make the upper bound 0x6e here, please -- the protocol docs go from 60 00 to 6E 6E, and we ought not pass along info for NRPNs outside the actual range. (Tho if distant memory of this code serves, nothing is actually going to go wrong, exactly, for nonexistent NRPNs here.)

this.emit('assign', makeNRPN(msb, lsb), vc, vf)
} else {
verboseLog(
`Unhandled MSB/LSB ${prettyByte(msb)}/${prettyByte(lsb)} in NRPN data message ${manyPrettyBytes(first, second, third, fourth)}`,
Expand Down
33 changes: 22 additions & 11 deletions src/mixer/mixer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
AssignNRPNCalculator,
BalanceNRPNCalculator,
forEachSourceSinkLevel,
forEachSourceSinkAssign,
LevelNRPNCalculator,
type SourceForSourceInMixAndLRForNRPN,
type SourceSinkForNRPN,
Expand Down Expand Up @@ -308,6 +309,10 @@ export class Mixer {
forEachSourceSinkLevel(model, getLevel)
forEachOutputLevel(model, getLevel)

// Request assign NRPNs as well so assignment variables are populated at startup

@jswalden jswalden Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I dislike the system of variables that only exist sometimes, and others you will into existence as setExtraVariable(s) (see elsewhere in module code). But the unfortunate reality is that gobs and gobs of variables are what the module has today, and some are Less Important than others and only come to life at later times.

I think this is better served by making these extra variables, that are only added/created as assign NRPN messages are received -- similar to what's done for pan/balance changes. My guess is that dynamic changes to assignments are closer to "as unlikely as pan/balance changes" than to "as likely as level or muting changes", and therefore defining these as extra variables is more sensible than defining them as variables that are always present.

...which, ugh, I guess means forEachSourceSink* didn't actually immediately need to be made generic, did it, and I devoted effort to something not actually important. Oh well.

const getAssign = (nrpn: NRPN<'assign'>) => buff.push(this.getNRPNValue(nrpn))
forEachSourceSinkAssign(model, getAssign)

const delayStatusRetrieval = instance.config.retrieveStatusAtStartup === RetrieveStatusAtStartup.Delayed

if (buff.length > 0 && this.#socket !== null) {
Expand Down Expand Up @@ -423,6 +428,14 @@ export class Mixer {
const variableValue = vcvfToReadablePanBalance(vc, vf)
instance.setExtraVariable(variableId, name, variableValue)
})
mixerChannelParser.on('assign', (nrpn: NRPN<'assign'>, vc: number, vf: number) => {
verboseLog(`Assign received: ${prettyNRPN(nrpn)}, VC=${prettyByte(vc)}, VF=${prettyByte(vf)}`)

const { MSB, LSB } = splitNRPN(nrpn)
const variableId = `assign_${MSB}.${LSB}`

@jswalden jswalden Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

At some point these variables -- or some other storage mechanism that makes use of variables' new (as of the 2.0 module API) ability to store any JSON value -- need to not directly incorporate NRPN or MSB/LSB, but instead should be like level_inputChannel1_group5 or something. I hesitate to create more mess like this to eventually require cleanup.

But the hard reality is that redoing all the variable naming/structuring will break every user anyway when it happens. And I don't yet have a way to work backwards from NRPN to the particular relationship it refers to. So I guess just doubling down on the existing approach is acceptable.

// Use '1'/'0' string values for Companion variables
instance.setVariableValues({ [variableId]: vf !== 0 ? '1' : '0' })

@jswalden jswalden Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Assignment being exposed as true or false seems preferable to me. Why do you think we ought expose them as strings containing number 1 or 0 instead? Boolean values are more amenable to use in expressions, which I intend at some point to convert the module to using everywhere.

})

return parseMidi(instance.config.midiChannel, verboseLog, tokenizer, mixerChannelParser)
}
Expand Down Expand Up @@ -583,6 +596,8 @@ export class Mixer {
*/
#assignToSink(source: number, active: boolean, sink: number, calc: AssignNRPNCalculator): NRPNDataMessage {
const nrpn = calc.calculate(source, sink)
const { MSB, LSB } = splitNRPN(nrpn)
this.#instance.setVariableValues({ [`assign_${MSB}.${LSB}`]: active ? '1' : '0' })
return this.#nrpnData(nrpn, 0, active ? 1 : 0)
}

Expand All @@ -608,16 +623,15 @@ export class Mixer {
active: boolean,
mixes: readonly MixOrLR[],
): void {
const mixNrpn = AssignNRPNCalculator.get(this.model, [sourceType, 'mix'])
const lrNrpn = AssignNRPNCalculator.get(this.model, [sourceType, 'lr'])
const mixNrpnCalc = AssignNRPNCalculator.get(this.model, [sourceType, 'mix'])
const lrNrpnCalc = AssignNRPNCalculator.get(this.model, [sourceType, 'lr'])

const commands = mixes.map((mixOrLR) => {
const commands: readonly number[][] = mixes.map((mixOrLR) => {

@jswalden jswalden Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If you're changing this at all (it shouldn't be necessary to change anything here), make it NRPNDataMessage[]

return mixOrLR === LR
? this.#assignToSink(source, active, 0, lrNrpn)
: this.#assignToSink(source, active, mixOrLR, mixNrpn)
? this.#assignToSink(source, active, 0, lrNrpnCalc)
: this.#assignToSink(source, active, mixOrLR, mixNrpnCalc)
})

// XXX

@jswalden jswalden Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please leave this as-is, as noted in the subsequent comment.

void this.sendCommands(commands)
}

Expand Down Expand Up @@ -646,13 +660,10 @@ export class Mixer {
sinks: readonly number[],
sourceSink: SourceSinkForNRPN<'assign'>,
): void {
const nrpn = AssignNRPNCalculator.get(this.model, sourceSink)
const calc = AssignNRPNCalculator.get(this.model, sourceSink)

const commands = sinks.map((sink) => {
return this.#assignToSink(source, active, sink, nrpn)
})
const commands = sinks.map((sink) => this.#assignToSink(source, active, sink, calc))

// XXX

@jswalden jswalden Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please leave this as-is -- it's alluding to how the promise returned here is dropped on the floor and if it rejects, that rejection goes entirely unhandled, a problem I haven't really figured out how to properly solve yet.

void this.sendCommands(commands)
}

Expand Down
24 changes: 24 additions & 0 deletions src/mixer/nrpn/source-to-sink.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,3 +389,27 @@ export function forEachSourceSinkLevel(model: Model, f: SourceSinkLevelFunctor):
}
}
}

type SourceSinkAssignFunctor = (nrpn: NRPN<'assign'>, sourceDesc: string, sinkDesc: string) => void

/**
* For each source-sink relationship with assign parameters, invoke the given
* function.
*/
export function forEachSourceSinkAssign(model: Model, f: SourceSinkAssignFunctor): void {

@jswalden jswalden Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is sensible-ish enough and all, but it's also avoidable copying. 🙂 I landed a rev generalizing the original function -- please rebase this atop that and make use of the new functionality.

...or wait, see later comments, I don't think you'll need to do this.

for (const [sourceType, sinks] of Object.entries(SourceToSinkParameterBase)) {
for (const sinkType of Object.entries(sinks).flatMap(([sinkType, params]) => {
return 'assign' in params ? sinkType : []
})) {
const sourceSink = [sourceType, sinkType] as SourceSinkForNRPN<'assign'>

const calc = AssignNRPNCalculator.get(model, sourceSink)
model.forEach(sourceSink[0], (source, _sourceLabel, sourceDesc) => {
model.forEach(sourceSink[1], (sink, _sinkLabel, sinkDesc) => {
const nrpn = calc.calculate(source, sink)
f(nrpn, sourceDesc, sinkDesc)
})
})
}
}
}
10 changes: 9 additions & 1 deletion src/variables.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { CompanionVariableDefinition } from '@companion-module/base'
import type { Model } from './mixer/model.js'
import { type NRPN, splitNRPN } from './mixer/nrpn/nrpn.js'
import { forEachOutputLevel } from './mixer/nrpn/output.js'
import { forEachSourceSinkLevel } from './mixer/nrpn/source-to-sink.js'
import { forEachSourceSinkLevel, forEachSourceSinkAssign } from './mixer/nrpn/source-to-sink.js'

/**
* The variable ID for the variable containing the last recalled scene
Expand Down Expand Up @@ -40,6 +40,14 @@ export function getVariables(model: Model): CompanionVariableDefinition[] {
addVariable(nrpn, `${sourceDesc} -> ${sinkDesc} Level`)
})

forEachSourceSinkAssign(model, (nrpn: NRPN<'assign'>, sourceDesc: string, sinkDesc: string) => {
const { MSB, LSB } = splitNRPN(nrpn)
variables.push({
name: `${sourceDesc} -> ${sinkDesc} Assigned`,
variableId: `assign_${MSB}.${LSB}`,
})
})

forEachOutputLevel(model, (nrpn, outputDesc) => {
addVariable(nrpn, `${outputDesc} Output Level`)
})
Expand Down