From 9de4ceb2ba49930c95176f32f2303a9ebd80b93d Mon Sep 17 00:00:00 2001 From: Charlemagne Santos Date: Fri, 31 Mar 2023 12:45:40 -0700 Subject: [PATCH] VBLOCKS-1111 | Fix sound definitions (#153) * VBLOCKS-1534 | Stop using deprecated RTCIceCandidateStats * VBLOCKS-1111 | Fix sound definitions * Tests and changelog * Update docs --- CHANGELOG.md | 2 + lib/twilio/audiohelper.ts | 84 ++++++++++++++++++++++++--------------- lib/twilio/device.ts | 20 ++-------- package.json | 3 +- tests/audiohelper.js | 30 +++++++++++++- tests/typecheck/index.ts | 15 +++++++ tests/unit/device.ts | 33 ++++++++++++++- 7 files changed, 134 insertions(+), 53 deletions(-) create mode 100644 tests/typecheck/index.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 629d70cf..da86fe2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Bug Fixes - Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/100) where a `TypeError` is thrown after rejecting a call then invoking `updateToken`. +- Fixed an [issue](https://github.com/twilio/twilio-voice.js/issues/14) where `device.audio.disconnect`, `device.audio.incoming` and `device.audio.outgoing` do not have the correct type definitions. + 2.3.2 (February 27, 2023) =================== diff --git a/lib/twilio/audiohelper.ts b/lib/twilio/audiohelper.ts index 843c9abf..2b41b6ed 100644 --- a/lib/twilio/audiohelper.ts +++ b/lib/twilio/audiohelper.ts @@ -90,6 +90,15 @@ class AudioHelper extends EventEmitter { */ private _audioContext?: AudioContext; + /** + * Whether each sound is enabled. + */ + private _enabledSounds: Record = { + [Device.SoundName.Disconnect]: true, + [Device.SoundName.Incoming]: true, + [Device.SoundName.Outgoing]: true, + }; + /** * The `getUserMedia()` function to use. */ @@ -172,10 +181,6 @@ class AudioHelper extends EventEmitter { this.isOutputSelectionSupported = isEnumerationSupported && isSetSinkSupported; this.isVolumeSupported = isAudioContextSupported; - if (options.enabledSounds) { - this._addEnabledSounds(options.enabledSounds); - } - if (this.isVolumeSupported) { this._audioContext = options.audioContext || options.AudioContext && new options.AudioContext(); if (this._audioContext) { @@ -287,6 +292,36 @@ class AudioHelper extends EventEmitter { } } + /** + * Enable or disable the disconnect sound. + * @param doEnable Passing `true` will enable the sound and `false` will disable the sound. + * Not passing this parameter will not alter the enable-status of the sound. + * @returns The enable-status of the sound. + */ + disconnect(doEnable?: boolean): boolean { + return this._maybeEnableSound(Device.SoundName.Disconnect, doEnable); + } + + /** + * Enable or disable the incoming sound. + * @param doEnable Passing `true` will enable the sound and `false` will disable the sound. + * Not passing this parameter will not alter the enable-status of the sound. + * @returns The enable-status of the sound. + */ + incoming(doEnable?: boolean): boolean { + return this._maybeEnableSound(Device.SoundName.Incoming, doEnable); + } + + /** + * Enable or disable the outgoing sound. + * @param doEnable Passing `true` will enable the sound and `false` will disable the sound. + * Not passing this parameter will not alter the enable-status of the sound. + * @returns The enable-status of the sound. + */ + outgoing(doEnable?: boolean): boolean { + return this._maybeEnableSound(Device.SoundName.Outgoing, doEnable); + } + /** * Set the MediaTrackConstraints to be applied on every getUserMedia call for new input * device audio. Any deviceId specified here will be ignored. Instead, device IDs should @@ -343,27 +378,6 @@ class AudioHelper extends EventEmitter { }); } - /** - * Merge the passed enabledSounds into {@link AudioHelper}. Currently used to merge the deprecated - * Device.sounds object onto the new {@link AudioHelper} interface. Mutates - * by reference, sharing state between {@link Device} and {@link AudioHelper}. - * @param enabledSounds - The initial sound settings to merge. - * @private - */ - private _addEnabledSounds(enabledSounds: { [name: string]: boolean }) { - function setValue(key: Device.ToggleableSound, value: boolean) { - if (typeof value !== 'undefined') { - enabledSounds[key] = value; - } - - return enabledSounds[key]; - } - - Object.keys(enabledSounds).forEach(key => { - (this as any)[key] = setValue.bind(null, key); - }); - } - /** * Get the index of an un-labeled Device. * @param mediaDeviceInfo @@ -407,6 +421,19 @@ class AudioHelper extends EventEmitter { }); } + /** + * Set whether the sound is enabled or not + * @param soundName + * @param doEnable + * @returns Whether the sound is enabled or not + */ + private _maybeEnableSound(soundName: Device.ToggleableSound, doEnable?: boolean): boolean { + if (typeof doEnable !== 'undefined') { + this._enabledSounds[soundName] = doEnable; + } + return this._enabledSounds[soundName]; + } + /** * Remove an input device from inputs * @param lostDevice @@ -669,13 +696,6 @@ namespace AudioHelper { */ audioContext?: AudioContext; - /** - * A Record of sounds. This is modified by reference, and is used to - * maintain backward-compatibility. This should be removed or refactored in 2.0. - * TODO: Remove / refactor in 2.0. (CLIENT-5302) - */ - enabledSounds?: Record; - /** * A custom MediaDevices instance to use. */ diff --git a/lib/twilio/device.ts b/lib/twilio/device.ts index 375f54a3..37d4dc74 100644 --- a/lib/twilio/device.ts +++ b/lib/twilio/device.ts @@ -354,15 +354,6 @@ class Device extends EventEmitter { */ private _edge: string | null = null; - /** - * Whether each sound is enabled. - */ - private _enabledSounds: Record = { - [Device.SoundName.Disconnect]: true, - [Device.SoundName.Incoming]: true, - [Device.SoundName.Outgoing]: true, - }; - /** * The name of the home region the {@link Device} is connected to. */ @@ -978,7 +969,7 @@ class Device extends EventEmitter { maxAverageBitrate: this._options.maxAverageBitrate, preflight: this._options.preflight, rtcConstraints: this._options.rtcConstraints, - shouldPlayDisconnect: () => this._enabledSounds.disconnect, + shouldPlayDisconnect: () => this.audio?.disconnect(), twimlParams, voiceEventSidGenerator: this._options.voiceEventSidGenerator, }, options); @@ -1003,7 +994,7 @@ class Device extends EventEmitter { this._audio._maybeStartPollingVolume(); } - if (call.direction === Call.CallDirection.Outgoing && this._enabledSounds.outgoing) { + if (call.direction === Call.CallDirection.Outgoing && this.audio?.outgoing()) { this._soundcache.get(Device.SoundName.Outgoing).play(); } @@ -1213,7 +1204,7 @@ class Device extends EventEmitter { this._publishNetworkChange(); }); - const play = (this._enabledSounds.incoming && !wasBusy) + const play = (this.audio?.incoming() && !wasBusy) ? () => this._soundcache.get(Device.SoundName.Incoming).play() : () => Promise.resolve(); @@ -1320,10 +1311,7 @@ class Device extends EventEmitter { this._updateSinkIds, this._updateInputStream, getUserMedia, - { - audioContext: Device.audioContext, - enabledSounds: this._enabledSounds, - }, + { audioContext: Device.audioContext }, ); this._audio.on('deviceChange', (lostActiveDevices: MediaDeviceInfo[]) => { diff --git a/package.json b/package.json index 6135e2eb..c9bfb9b5 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "url": "git@github.com:twilio/twilio-voice.js.git" }, "scripts": { - "build": "npm-run-all clean build:constants build:errors docs:ts build:es5 build:ts build:dist build:dist-min", + "build": "npm-run-all clean build:constants build:errors docs:ts build:es5 build:ts build:dist build:dist-min test:typecheck", "build:errors": "node ./scripts/errors.js", "build:es5": "rimraf ./es5 && babel lib -d es5", "build:dev": "ENV=dev npm run build", @@ -55,6 +55,7 @@ "test:integration": "karma start $PWD/karma.conf.ts", "test:network": "node ./scripts/karma.js $PWD/karma.network.conf.ts", "test:selenium": "mocha tests/browser/index.js", + "test:typecheck": "./node_modules/typescript/bin/tsc tests/typecheck/index.ts --noEmit", "test:unit": "nyc mocha -r ts-node/register ./tests/index.ts", "test:webpack": "cd ./tests/webpack && npm install && npm test" }, diff --git a/tests/audiohelper.js b/tests/audiohelper.js index 4c27f3de..54699418 100644 --- a/tests/audiohelper.js +++ b/tests/audiohelper.js @@ -297,7 +297,33 @@ describe('AudioHelper', () => { }); }); - describe('setAudioConstraints', () => { + ['disconnect', 'incoming', 'outgoing'].forEach(soundName => { + describe(`.${soundName}`, () => { + let testFn; + + beforeEach(() => { + testFn = audio[soundName].bind(audio); + }); + + it('should return true as default', () => { + assert.strictEqual(testFn(), true); + }); + + it('should return false after setting to false', () => { + assert.strictEqual(testFn(false), false); + assert.strictEqual(testFn(), false); + }); + + it('should return true after setting to true', () => { + assert.strictEqual(testFn(false), false); + assert.strictEqual(testFn(), false); + assert.strictEqual(testFn(true), true); + assert.strictEqual(testFn(), true); + }); + }); + }); + + describe('.setAudioConstraints', () => { context('when no input device is active', () => { it('should set .audioConstraints', () => { audio.setAudioConstraints({ foo: 'bar' }); @@ -328,7 +354,7 @@ describe('AudioHelper', () => { }); }); - describe('unsetAudioConstraints', () => { + describe('.unsetAudioConstraints', () => { beforeEach(() => { audio.setAudioConstraints({ foo: 'bar' }); }); diff --git a/tests/typecheck/index.ts b/tests/typecheck/index.ts new file mode 100644 index 00000000..74f3d1a0 --- /dev/null +++ b/tests/typecheck/index.ts @@ -0,0 +1,15 @@ +import { Device } from '../../'; + +(async () => { + const device: Device = new Device('foo', {}); + + await device.register(); + + const call = await device.connect({ + params: { To: 'foo' } + }); + + device.audio?.disconnect(false); + device.audio?.incoming(false); + device.audio?.outgoing(false); +}); diff --git a/tests/unit/device.ts b/tests/unit/device.ts index 528b1f08..2bf3a9bc 100644 --- a/tests/unit/device.ts +++ b/tests/unit/device.ts @@ -26,6 +26,7 @@ describe('Device', function() { let clock: SinonFakeTimers; let connectOptions: Record | undefined; let device: Device; + let enabledSounds: Record; let pstream: any; let publisher: any; let stub: SinonStubbedInstance; @@ -40,7 +41,11 @@ describe('Device', function() { const AudioHelper = (_updateSinkIds: Function, _updateInputStream: Function) => { updateInputStream = _updateInputStream; updateSinkIds = _updateSinkIds; - return audioHelper = createEmitterStub(require('../../lib/twilio/audiohelper').default); + const audioHelper = createEmitterStub(require('../../lib/twilio/audiohelper').default); + audioHelper.disconnect = () => enabledSounds[Device.SoundName.Disconnect]; + audioHelper.incoming = () => enabledSounds[Device.SoundName.Incoming]; + audioHelper.outgoing = () => enabledSounds[Device.SoundName.Outgoing]; + return audioHelper; }; const Call = (_?: any, _connectOptions?: Record) => { connectOptions = _connectOptions; @@ -63,6 +68,11 @@ describe('Device', function() { }); beforeEach(() => { + enabledSounds = { + [Device.SoundName.Disconnect]: true, + [Device.SoundName.Incoming]: true, + [Device.SoundName.Outgoing]: true, + }; pstream = null; publisher = null; clock = sinon.useFakeTimers(Date.now()); @@ -227,6 +237,16 @@ describe('Device', function() { activeCall.emit('accept'); sinon.assert.calledOnce(spy.play); }); + + it('should not play outgoing sound after accepted if disabled', async () => { + enabledSounds[Device.SoundName.Outgoing] = false; + const spy: any = { play: sinon.spy() }; + device['_soundcache'].set(Device.SoundName.Outgoing, spy); + await device.connect(); + activeCall._direction = 'OUTGOING'; + activeCall.emit('accept'); + sinon.assert.notCalled(spy.play); + }); }); describe('.destroy()', () => { @@ -670,7 +690,7 @@ describe('Device', function() { }); }); - it('should play the incoming sound', async () => { + it('should play the incoming sound if enabled', async () => { const spy = { play: sinon.spy() }; device['_soundcache'].set(Device.SoundName.Incoming, spy); pstream.emit('invite', { callsid: 'foo', sdp: 'bar' }); @@ -678,6 +698,15 @@ describe('Device', function() { sinon.assert.calledOnce(spy.play); }); + it('should not play the incoming sound if disabled', async () => { + enabledSounds[Device.SoundName.Incoming] = false; + const spy = { play: sinon.spy() }; + device['_soundcache'].set(Device.SoundName.Incoming, spy); + pstream.emit('invite', { callsid: 'foo', sdp: 'bar' }); + await clock.tickAsync(0); + sinon.assert.notCalled(spy.play); + }); + context('when allowIncomingWhileBusy is true', () => { beforeEach(async () => { device = new Device(token, { ...setupOptions, allowIncomingWhileBusy: true });