Skip to content

Commit

Permalink
chore: Remove the label connection option, because `InstanceBase.la…
Browse files Browse the repository at this point in the history
…bel` exists and also is always accurate.
  • Loading branch information
jswalden committed Nov 22, 2024
1 parent 0fd2cb7 commit b3bb1f3
Show file tree
Hide file tree
Showing 8 changed files with 131 additions and 38 deletions.
6 changes: 3 additions & 3 deletions src/actions/output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ export function outputActions(

return {
...options,
showvar: `$(${instance.options.connectionLabel}:pan_${MSB}.${LSB})`,
showvar: `$(${instance.label}:pan_${MSB}.${LSB})`,
}
},
subscribe: async (_action) => {
Expand Down Expand Up @@ -509,7 +509,7 @@ export function outputActions(

return {
...options,
showvar: `$(${instance.options.connectionLabel}:pan_${MSB}.${LSB})`,
showvar: `$(${instance.label}:pan_${MSB}.${LSB})`,
}
},
subscribe: async ({ options }) => {
Expand Down Expand Up @@ -558,7 +558,7 @@ export function outputActions(

return {
...options,
showvar: `$(${instance.options.connectionLabel}:pan_${MSB}.${LSB})`,
showvar: `$(${instance.label}:pan_${MSB}.${LSB})`,
}
},
subscribe: async ({ options }) => {
Expand Down
12 changes: 6 additions & 6 deletions src/actions/pan-balance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export function panBalanceActions(

return {
...options,
showvar: `$(${instance.options.connectionLabel}:pan_${MSB}.${LSB})`,
showvar: `$(${instance.label}:pan_${MSB}.${LSB})`,
}
},
subscribe: async (action) => {
Expand Down Expand Up @@ -224,7 +224,7 @@ export function panBalanceActions(

return {
...options,
showvar: `$(${instance.options.connectionLabel}:pan_${MSB}.${LSB})`,
showvar: `$(${instance.label}:pan_${MSB}.${LSB})`,
}
},
subscribe: async (action) => {
Expand Down Expand Up @@ -303,7 +303,7 @@ export function panBalanceActions(

return {
...options,
showvar: `$(${instance.options.connectionLabel}:pan_${MSB}.${LSB})`,
showvar: `$(${instance.label}:pan_${MSB}.${LSB})`,
}
},
subscribe: async (action) => {
Expand Down Expand Up @@ -382,7 +382,7 @@ export function panBalanceActions(

return {
...options,
showvar: `$(${instance.options.connectionLabel}:pan_${MSB}.${LSB})`,
showvar: `$(${instance.label}:pan_${MSB}.${LSB})`,
}
},
subscribe: async (action) => {
Expand Down Expand Up @@ -466,7 +466,7 @@ export function panBalanceActions(

return {
...options,
showvar: `$(${instance.options.connectionLabel}:pan_${MSB}.${LSB})`,
showvar: `$(${instance.label}:pan_${MSB}.${LSB})`,
}
},
subscribe: async (action) => {
Expand Down Expand Up @@ -551,7 +551,7 @@ export function panBalanceActions(

return {
...options,
showvar: `$(${instance.options.connectionLabel}:pan_${MSB}.${LSB})`,
showvar: `$(${instance.label}:pan_${MSB}.${LSB})`,
}
},
subscribe: async (action) => {
Expand Down
46 changes: 42 additions & 4 deletions src/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,22 @@ import {
addModelOptionToConfig,
configIsMissingLabel,
configIsMissingModel,
configUnnecessarilySpecifiesLabel,
removeLabelOptionFromConfig,
type SQInstanceConfig,
} from './config.js'

describe('config upgrade to specify a missing model', () => {
test('config without model', () => {
const configMissingModel = {
const configMissingModel: SQInstanceConfig = {
host: '127.0.0.1',
level: 'LinearTaper',
talkback: 0,
midich: 0,
status: 'full',
label: 'SQ',
verbose: false,
} as SQInstanceConfig
}

expect(configIsMissingModel(configMissingModel)).toBe(true)

Expand Down Expand Up @@ -62,15 +64,15 @@ describe('config upgrade to specify a missing model', () => {

describe('config upgrade to specify a missing label', () => {
test('config without label', () => {
const configMissingLabel = {
const configMissingLabel: SQInstanceConfig = {
host: '127.0.0.1',
model: 'SQ5',
level: 'LinearTaper',
talkback: 0,
midich: 0,
status: 'full',
verbose: false,
} as SQInstanceConfig
}

expect(configIsMissingLabel(configMissingLabel)).toBe(true)

Expand All @@ -96,3 +98,39 @@ describe('config upgrade to specify a missing label', () => {
expect(configWithLabelSQ5.label).toBe('sq5')
})
})

describe('config upgrade to remove an unnecessary label', () => {
test('config without label', () => {
const configMissingLabel: SQInstanceConfig = {
host: '127.0.0.1',
model: 'SQ5',
level: 'LinearTaper',
talkback: 0,
midich: 0,
status: 'full',
verbose: false,
}

expect(configUnnecessarilySpecifiesLabel(configMissingLabel)).toBe(false)
})

test('config with label', () => {
const configWithLabel: SQInstanceConfig = {
host: '127.0.0.1',
model: 'SQ5',
level: 'LinearTaper',
talkback: 0,
midich: 0,
status: 'full',
verbose: false,
label: 'SQ',
}

expect(configUnnecessarilySpecifiesLabel(configWithLabel)).toBe(true)

removeLabelOptionFromConfig(configWithLabel)

expect(configUnnecessarilySpecifiesLabel(configWithLabel)).toBe(false)
expect('label' in configWithLabel).toBe(false)
})
})
23 changes: 15 additions & 8 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,26 @@ export function configIsMissingLabel(config: SQInstanceConfig | null): config is
}

/**
* Add the 'label' option (defaulting to SQ) to a config that's missing one.
* Add the `'label'` option (defaulting to SQ) to a config that's missing one.
*/
export function addLabelOptionToConfig(config: SQInstanceConfig): void {
config.label = DefaultConnectionLabel
}

/**
* Determine whether the given instance config specifies a `'label'` property
* that is unnecessary because `InstanceBase.label` is the real deal and is
* always up-to-date.
*/
export function configUnnecessarilySpecifiesLabel(config: SQInstanceConfig | null): config is SQInstanceConfig {
return config !== null && 'label' in config
}

/** Remove the `'label'` option from a config that has it. */
export function removeLabelOptionFromConfig(config: SQInstanceConfig): void {
delete config.label
}

function createDefaultTalkbackChannelOption(): SomeCompanionConfigField {
// The number of input channels depends on how many input channels the
// user's chosen SQ model has. Currently all SQs have the same number of
Expand Down Expand Up @@ -133,13 +147,6 @@ export function GetConfigFields(): SomeCompanionConfigField[] {
{ id: 'nosts', label: 'Not at startup' },
],
},
{
type: 'textinput',
id: 'label',
label: 'Connection label (for displaying pan/balance variables correctly)',
width: 6,
default: DefaultConnectionLabel,
},
{
type: 'checkbox',
id: 'verbose',
Expand Down
21 changes: 21 additions & 0 deletions src/instance.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ export class sqInstance extends InstanceBase {
/** @type {Mixer | null} */
mixer = null

/**
* The last label specified for this instance, or `null` if there wasn't a
* last label.
*
* @type {string | null}
*/
#lastLabel = null

/**
* Construct an `sqInstance`.
*
Expand Down Expand Up @@ -113,6 +121,17 @@ export class sqInstance extends InstanceBase {
this.options = newOptions

if (canUpdateOptionsWithoutRestarting(oldOptions, newOptions)) {
const label = this.label
if (label !== this.#lastLabel) {
// The instance label might be altered just before
// `configUpdated` is called. The instance label is used in the
// "Learn" operation for some actions -- and it'll always be
// up-to-date in these uses. But it's also hardcoded in some
// presets, so if the label changes, we must redefine presets
// even if we don't have to restart the connection.
this.#lastLabel = label
this.setPresetDefinitions(getPresets(this, this.mixer.model))
}
return
}

Expand All @@ -128,6 +147,8 @@ export class sqInstance extends InstanceBase {
this.initVariableDefinitions(model)
this.setActionDefinitions(getActions(this, mixer, choices))
this.setFeedbackDefinitions(getFeedbacks(mixer, choices))

this.#lastLabel = this.label
this.setPresetDefinitions(getPresets(this, model))

//this.checkVariables();
Expand Down
16 changes: 1 addition & 15 deletions src/options.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { type ModelId, DefaultModel } from './mixer/models.js'
import type { FaderLaw } from './mixer/mixer.js'
import { RetrieveStatusAtStartup } from './mixer/mixer.js'
import { DefaultConnectionLabel, type SQInstanceConfig } from './config.js'
import type { SQInstanceConfig } from './config.js'
import { Regex } from '@companion-module/base'

/** Options information controlling the operation of a mixer instance. */
Expand Down Expand Up @@ -40,9 +40,6 @@ export type SQInstanceOptions = {
* logging is enabled.
*/
verbose: boolean

/** The label used to identify this Companion instance/connection. */
connectionLabel: string
}

function toHost(host: SQInstanceConfig['host']): string | null {
Expand Down Expand Up @@ -109,10 +106,6 @@ function toVerbose(verbose: SQInstanceConfig['verbose']): boolean {
return Boolean(verbose)
}

function toConnectionLabel(label: SQInstanceConfig['label']): string {
return typeof label === 'undefined' ? DefaultConnectionLabel : String(label)
}

/** Compute instance options from instance configuration info. */
export function optionsFromConfig({
// Comments indicate the expected types of the various config fields.
Expand All @@ -123,7 +116,6 @@ export function optionsFromConfig({
midich, // number
status, // string
verbose, // boolean
label, // string
}: SQInstanceConfig): SQInstanceOptions {
return {
host: toHost(host),
Expand All @@ -133,7 +125,6 @@ export function optionsFromConfig({
midiChannel: toMidiChannel(midich),
retrieveStatusAtStartup: toRetrieveStatusAtStartup(status),
verbose: toVerbose(verbose),
connectionLabel: toConnectionLabel(label),
}
}

Expand All @@ -151,7 +142,6 @@ export function noConnectionOptions(): SQInstanceOptions {
midiChannel: 0,
retrieveStatusAtStartup: RetrieveStatusAtStartup.Fully,
verbose: false,
connectionLabel: DefaultConnectionLabel,
}
}

Expand Down Expand Up @@ -202,10 +192,6 @@ export function canUpdateOptionsWithoutRestarting(
// retrieval is extremely slow (particularly if the instance debug log is
// open.)

// The connection label is only used to expose the variable names for
// pan/balance variables that are "Learn"ed at runtime, so don't restart for
// a label change.

// Otherwise we can update options without restarting.
return true
}
2 changes: 1 addition & 1 deletion src/presets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export function getPresets(instance: sqInstance, model: Model): CompanionPresetD

createtMuteLevel(
`Mt+dB CH-${mixLabel}`,
`${channelLabel}\\n${mixLabel}\\n$(${instance.options.connectionLabel}:level_${MSB}.${LSB}) dB`,
`${channelLabel}\\n${mixLabel}\\n$(${instance.label}:level_${MSB}.${LSB}) dB`,
'MuteInputChannel',
channel,
mix,
Expand Down
43 changes: 42 additions & 1 deletion src/upgrades.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@ import {
type CompanionUpgradeContext,
EmptyUpgradeScript,
} from '@companion-module/base'
import { configIsMissingLabel, configIsMissingModel, DefaultConnectionLabel, type SQInstanceConfig } from './config.js'
import {
configIsMissingLabel,
configIsMissingModel,
configUnnecessarilySpecifiesLabel,
DefaultConnectionLabel,
removeLabelOptionFromConfig,
type SQInstanceConfig,
} from './config.js'
import { DefaultModel } from './mixer/models.js'
import {
convertOldLevelToOutputActionToSinkSpecific,
Expand Down Expand Up @@ -165,13 +172,47 @@ function RewriteCombinedOutputPanBalanceActionsToSinkSpecificOutputPanBalanceAct
return result
}

/**
* This module used to have a `'label'` option, in which the user was expected
* to (re-)specify the instance label. This label was then used in the "Learn"
* operation for various actions, as well as in various preset definitions.
*
* But it turns out the instance label is accessible as `InstanceBase.label`
* which is always up-to-date, so there's no point in having the config option.
*
* This upgrade script removes the `'label'` option from configs that have it.
*/
function RemoveUnnecessaryConnectionLabel(
_context: CompanionUpgradeContext<SQInstanceConfig>,
props: CompanionStaticUpgradeProps<SQInstanceConfig>,
): CompanionStaticUpgradeResult<SQInstanceConfig> {
const result: CompanionStaticUpgradeResult<SQInstanceConfig> = {
updatedConfig: null,
updatedActions: [],
updatedFeedbacks: [],
}

const oldConfig = props.config
if (configUnnecessarilySpecifiesLabel(oldConfig)) {
removeLabelOptionFromConfig(oldConfig)
result.updatedConfig = oldConfig
}

return result
}

export const UpgradeScripts = [
EmptyUpgradeScript,
CoalesceSceneRecallActions,
EnsureModel,
EnsureConnectionLabel,
RewriteCombinedOutputLevelActionsToSinkSpecificOutputLevelActions,
RewriteCombinedOutputPanBalanceActionsToSinkSpecificOutputPanBalanceActions,
// ...yes, we added the `'label'` config option above because we thought it
// was the only way to get the instance label, and now we're removing it
// because there in fact *is* a way to get that label without requiring that
// users redundantly specify it. So it goes.
RemoveUnnecessaryConnectionLabel,
]

/*
Expand Down

0 comments on commit b3bb1f3

Please sign in to comment.