diff --git a/CHANGELOG.md b/CHANGELOG.md index 45e1429..3039507 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ ## [6.0.0] + +fix: `onAdjustVolume` now receives the relative delta from the request's `volume` field instead of the nonexistent `volumeDelta` field. Its optional third argument exposes `volumeDefault` when supplied. Absolute `setVolume` requests continue to use `onVolume(deviceId, volume)`. `onAdjustVolume` can now return `{ success, volume }` to report the volume after the adjustment, which SinricPro stores as the device's absolute level; returning a plain boolean still echoes the delta. feat: Local control - devices answer signed commands over the LAN, so they keep working while SinricPro is unreachable. diff --git a/examples/speaker/index.ts b/examples/speaker/index.ts index 5e52882..9a35bd2 100644 --- a/examples/speaker/index.ts +++ b/examples/speaker/index.ts @@ -59,7 +59,8 @@ async function main() { console.log(`\n[Volume] Device ${deviceId} adjust by ${delta > 0 ? '+' : ''}${delta}`); speakerState.volume = Math.max(0, Math.min(100, speakerState.volume + delta)); console.log(` New volume: ${speakerState.volume}`); - return true; + // Report the adjusted level; SinricPro stores it as the device's volume. + return { success: true, volume: speakerState.volume }; }); // Mute control diff --git a/examples/tv/index.ts b/examples/tv/index.ts index 23be3c9..98af309 100644 --- a/examples/tv/index.ts +++ b/examples/tv/index.ts @@ -56,7 +56,8 @@ async function main() { console.log(`\n[Volume] Device ${deviceId} adjust by ${delta > 0 ? '+' : ''}${delta}`); tvState.volume = Math.max(0, Math.min(100, tvState.volume + delta)); console.log(` New volume: ${tvState.volume}`); - return true; + // Report the adjusted level; SinricPro stores it as the device's volume. + return { success: true, volume: tvState.volume }; }); // Mute control diff --git a/src/capabilities/VolumeController.ts b/src/capabilities/VolumeController.ts index 7604aa6..f533163 100644 --- a/src/capabilities/VolumeController.ts +++ b/src/capabilities/VolumeController.ts @@ -15,13 +15,23 @@ export type VolumeCallback = ( volume: number ) => Promise | CallbackResult; +/** + * Superset of CallbackResult: `volume` reports the device's volume after the + * adjustment. Omitting it echoes the delta back, which the server would then + * store as the absolute level. + */ +export type AdjustVolumeResult = boolean | { success: boolean; message?: string; volume?: number }; + export type AdjustVolumeCallback = ( deviceId: string, - volumeDelta: number -) => Promise | CallbackResult; + volumeDelta: number, + volumeDefault?: boolean +) => Promise | AdjustVolumeResult; export interface IVolumeController { + /** Handle setVolume requests with an absolute volume. */ onVolume(callback: VolumeCallback): void; + /** Handle relative volume changes; the optional third argument preserves volumeDefault. */ onAdjustVolume(callback: AdjustVolumeCallback): void; sendVolumeEvent(volume: number, cause?: string): Promise; } @@ -78,11 +88,20 @@ export function VolumeController>(Base: T } if (request.action === 'adjustVolume' && this.adjustVolumeCallback) { - const volumeDelta = request.requestValue.volumeDelta; - const result = await this.adjustVolumeCallback(this.getDeviceId(), volumeDelta); + // The protocol uses "volume" for both absolute values and relative deltas. + const volumeDelta = request.requestValue.volume; + const result = await this.adjustVolumeCallback( + this.getDeviceId(), + volumeDelta, + request.requestValue.volumeDefault + ); // Handle both boolean and object return types let success: boolean; + // The server stores the response volume as the device's absolute level, so + // report the adjusted volume when the callback supplies one. + let volume = volumeDelta; + if (typeof result === 'boolean') { success = result; } else { @@ -90,10 +109,13 @@ export function VolumeController>(Base: T if (result.message) { request.errorMessage = result.message; } + if (result.volume !== undefined) { + volume = result.volume; + } } if (success) { - request.responseValue.volume = volumeDelta; + request.responseValue.volume = volume; } return success; diff --git a/test/unit/VolumeController.test.ts b/test/unit/VolumeController.test.ts new file mode 100644 index 0000000..22875d8 --- /dev/null +++ b/test/unit/VolumeController.test.ts @@ -0,0 +1,111 @@ +import { SinricProTV } from '../../src/devices/SinricProTV'; +import { SinricProSpeaker } from '../../src/devices/SinricProSpeaker'; +import type { SinricProRequest } from '../../src/core/types'; + +function volumeRequest(action: string, volume: number, volumeDefault?: boolean): SinricProRequest { + return { + action, + instance: '', + requestValue: { volume, ...(volumeDefault === undefined ? {} : { volumeDefault }) }, + responseValue: {}, + }; +} + +describe.each([ + ['TV', SinricProTV], + ['Speaker', SinricProSpeaker], +] as const)('VolumeController on %s', (_name, createDevice) => { + it.each([0, 50, 100])('dispatches absolute volume %s only to onVolume', async (volume) => { + const device = createDevice('test-device'); + const absolute = jest.fn(() => true); + const relative = jest.fn(() => true); + device.onVolume(absolute); + device.onAdjustVolume(relative); + const request = volumeRequest('setVolume', volume); + + expect(await device.handleRequest(request)).toBe(true); + expect(absolute).toHaveBeenCalledTimes(1); + expect(absolute).toHaveBeenCalledWith('test-device', volume); + expect(relative).not.toHaveBeenCalled(); + expect(request.responseValue).toEqual({ volume }); + }); + + it.each([-5, 0, 5])('dispatches relative volume %s only to onAdjustVolume', async (delta) => { + const device = createDevice('test-device'); + const absolute = jest.fn(() => true); + let currentVolume = 50; + const relative = jest.fn(async (_deviceId: string, volumeDelta: number) => { + currentVolume += volumeDelta; + return { success: true, volume: currentVolume }; + }); + device.onVolume(absolute); + device.onAdjustVolume(relative); + const request = volumeRequest('adjustVolume', delta, false); + + expect(await device.handleRequest(request)).toBe(true); + expect(relative).toHaveBeenCalledTimes(1); + expect(relative).toHaveBeenCalledWith('test-device', delta, false); + expect(currentVolume).toBe(50 + delta); + expect(absolute).not.toHaveBeenCalled(); + // The server stores this as the device's absolute level, not the delta. + expect(request.responseValue).toEqual({ volume: 50 + delta }); + }); + + it.each([-5, 0, 5])('echoes the %s delta when no volume is reported', async (delta) => { + const device = createDevice('test-device'); + const request = volumeRequest('adjustVolume', delta, false); + device.onAdjustVolume(() => true); + + expect(await device.handleRequest(request)).toBe(true); + expect(request.responseValue).toEqual({ volume: delta }); + }); + + it('reports volume 0 rather than falling back to the delta', async () => { + const device = createDevice('test-device'); + const request = volumeRequest('adjustVolume', -50, false); + device.onAdjustVolume(() => ({ success: true, volume: 0 })); + + expect(await device.handleRequest(request)).toBe(true); + expect(request.responseValue).toEqual({ volume: 0 }); + }); + + it('ignores a reported volume when the callback fails', async () => { + const device = createDevice('test-device'); + const request = volumeRequest('adjustVolume', 5, false); + device.onAdjustVolume(() => ({ success: false, message: 'Amp offline', volume: 55 })); + + expect(await device.handleRequest(request)).toBe(false); + expect(request.errorMessage).toBe('Amp offline'); + expect(request.responseValue).toEqual({}); + }); + + it.each([true, false, undefined])('preserves volumeDefault=%s', async (volumeDefault) => { + const device = createDevice('test-device'); + const callback = jest.fn(() => true); + device.onAdjustVolume(callback); + + expect(await device.handleRequest(volumeRequest('adjustVolume', -5, volumeDefault))).toBe(true); + expect(callback).toHaveBeenCalledWith('test-device', -5, volumeDefault); + }); + + it.each(['setVolume', 'adjustVolume'])('preserves callback errors for %s', async (action) => { + const device = createDevice('test-device'); + const callback = jest.fn(async () => ({ success: false, message: 'Receiver unavailable' })); + device.onVolume(callback); + device.onAdjustVolume(callback); + const request = volumeRequest(action, 5); + + expect(await device.handleRequest(request)).toBe(false); + expect(callback).toHaveBeenCalledTimes(1); + expect(request.errorMessage).toBe('Receiver unavailable'); + expect(request.responseValue).toEqual({}); + }); + + it.each(['setVolume', 'adjustVolume'])( + 'returns false without a callback for %s', + async (action) => { + const device = createDevice('test-device'); + expect(await device.handleRequest(volumeRequest(action, 5))).toBe(false); + } + ); +});