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 volume setting to talk on speaker #2179

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions front/src/config/i18n/de.json
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,7 @@
"description": "Diese Aktion lässt Gladys auf dem ausgewählten Lautsprecher sprechen.",
"needGladysPlus": "Erfordert Gladys Plus, da die Text-to-Speech-APIs kostenpflichtig sind.",
"deviceLabel": "Lautsprecher",
"volumeLabel": "Volumen",
"textLabel": "Nachricht zum Sprechen auf dem Lautsprecher",
"variablesExplanation": "Um eine Variable einzufügen, gib \"{{\" ein. Hinweis: Du musst vor diesem Block eine Variable in einer \"Letzten Zustand abrufen\"-Aktion definiert haben."
}
Expand Down
1 change: 1 addition & 0 deletions front/src/config/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,7 @@
"description": "This action will make Gladys speak on the selected speaker.",
"needGladysPlus": "Requires Gladys Plus as Text-To-Speech APIs are paid.",
"deviceLabel": "Speaker",
"volumeLabel": "Volume",
"textLabel": "Message to speak on the speaker",
"variablesExplanation": "To inject a variable, type '{{ '. Note: You must have defined a variable beforehand in a 'Retrieve Last State' action placed before this message block."
}
Expand Down
1 change: 1 addition & 0 deletions front/src/config/i18n/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -2021,6 +2021,7 @@
"description": "Cette action fera parler Gladys sur l'enceinte sélectionnée.",
"needGladysPlus": "Nécessite Gladys Plus car les API de \"Text To Speech\" sont payantes.",
"deviceLabel": "Enceinte",
"volumeLabel": "Volume",
"textLabel": "Message à dire sur l'enceinte",
"variablesExplanation": "Pour injecter une variable, tapez '{{ '. Attention, vous devez avoir défini une variable auparavant dans une action 'Récupérer le dernier état' placé avant ce bloc message."
}
Expand Down
21 changes: 21 additions & 0 deletions front/src/routes/scene/edit-scene/actions/PlayNotification.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class PlayNotification extends Component {
console.error(e);
}
};
updateVolume = e => {
this.props.updateActionProperty(this.props.columnIndex, this.props.index, 'volume', e.target.value);
};
updateText = text => {
this.props.updateActionProperty(this.props.columnIndex, this.props.index, 'text', text);
};
Expand Down Expand Up @@ -84,6 +87,24 @@ class PlayNotification extends Component {
onChange={this.handleDeviceChange}
/>
</div>
<div class="form-group">
<label class="form-label">
<Text id="editScene.actionsCard.playNotification.volumeLabel" />
<span class="form-required">
<Text id="global.requiredField" />
</span>
</label>
<input type="text" class="form-control" value={props.action.volume} disabled />
<input
type="range"
value={props.action.volume}
onChange={this.updateVolume}
class="form-control custom-range"
step="1"
min={0}
max={100}
/>
</div>
<div class="form-group">
<label class="form-label">
<Text id="editScene.actionsCard.playNotification.textLabel" />{' '}
Expand Down
5 changes: 3 additions & 2 deletions server/lib/device/device.setValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,19 @@ const { NotFoundError } = require('../../utils/coreErrors');
* @param {object} device - The device to control.
* @param {object} deviceFeature - The deviceFeature to control.
* @param {string|number} value - The new state to set.
* @param {object} options - Optional configs.
* @example
* device.setValue(device, deviceFeature);
*/
async function setValue(device, deviceFeature, value) {
async function setValue(device, deviceFeature, value, options = {}) {
const service = this.serviceManager.getService(device.service.name);
if (service === null) {
throw new NotFoundError(`Service ${device.service.name} was not found.`);
}
if (typeof get(service, 'device.setValue') !== 'function') {
throw new NotFoundError(`Function device.setValue in service ${device.service.name} does not exist.`);
}
await service.device.setValue(device, deviceFeature, value);
await service.device.setValue(device, deviceFeature, value, options);
// If device has feedback, the feedback will be sent and saved
// If value is a string, no need to save it
// @ts-ignore
Expand Down
2 changes: 1 addition & 1 deletion server/lib/scene/scene.actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ const actionsFunc = {
// Get TTS URL
const { url } = await self.gateway.getTTSApiUrl({ text: messageWithVariables });
// Play TTS Notification on device
await self.device.setValue(device, deviceFeature, url);
await self.device.setValue(device, deviceFeature, url, { volume: action.volume });
},
[ACTIONS.SMS.SEND]: async (self, action, scope) => {
const freeMobileService = self.service.getService('free-mobile');
Expand Down
4 changes: 4 additions & 0 deletions server/models/scene.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ const actionSchema = Joi.array().items(
message: Joi.string().allow(''),
blinking_time: Joi.number(),
blinking_speed: Joi.string().valid('slow', 'medium', 'fast'),
volume: Joi.number()
.integer()
.max(100)
.min(0),
}),
),
);
Expand Down
7 changes: 4 additions & 3 deletions server/services/airplay/lib/airplay.setValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ const logger = require('../../../utils/logger');
* @param {object} device - Updated Gladys device.
* @param {object} deviceFeature - Updated Gladys device feature.
* @param {string|number} value - The new device feature value.
* @param {object} options - Optional configs.
* @example
* setValue(device, deviceFeature, 0);
* setValue(device, deviceFeature, 0, 30);
*/
async function setValue(device, deviceFeature, value) {
async function setValue(device, deviceFeature, value, options) {
const deviceName = device.external_id.split(':')[1];
const ipAddress = this.deviceIpAddresses.get(deviceName);
if (!ipAddress) {
Expand All @@ -19,7 +20,7 @@ async function setValue(device, deviceFeature, value) {
if (deviceFeature.type === DEVICE_FEATURE_TYPES.MUSIC.PLAY_NOTIFICATION) {
const client = new this.Airtunes();
const airplayDevice = client.add(ipAddress, {
volume: 70,
volume: options?.volume || 70,
});
let decodeProcess;

Expand Down
20 changes: 17 additions & 3 deletions server/services/google-cast/lib/google_cast.setValue.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
const { promisify } = require('util');

const { DEVICE_FEATURE_TYPES } = require('../../../utils/constants');
const logger = require('../../../utils/logger');
/**
* @description Send the new device value over device protocol.
* @param {object} device - Updated Gladys device.
* @param {object} deviceFeature - Updated Gladys device feature.
* @param {string|number} value - The new device feature value.
* @param {object} options - Optional configs.
* @example
* setValue(device, deviceFeature, 0);
*/
async function setValue(device, deviceFeature, value) {
async function setValue(device, deviceFeature, value, options) {
const deviceName = device.external_id.split(':')[1];
const ipAddress = this.deviceIpAddresses.get(deviceName);
if (!ipAddress) {
Expand All @@ -19,8 +22,16 @@
const { Client, DefaultMediaReceiver } = this.googleCastLib;
const client = new Client();

client.connect(ipAddress, () => {
client.connect(ipAddress, async () => {
logger.debug('Google Cast Connected, launching app ...');
const getVolume = promisify(client.getVolume.bind(client));
const setVolume = promisify(client.setVolume.bind(client));

const { level } = await getVolume();

if (options.volume) {
await setVolume({ level: options.volume / 100 });
}

Copy link
Contributor

Choose a reason for hiding this comment

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

You're not waiting for the callback to finish, are you sure the volume will be changed when client.launch will be started ?

You can use promisify in utils to turn this in a promise for example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍 . Volume is set immediatly, when we play audio from url it needs time to load it, so I thought is was not necessary

client.launch(DefaultMediaReceiver, (err, player) => {
const media = {
Expand All @@ -38,8 +49,11 @@
},
};

player.on('status', (status) => {
player.on('status', async (status) => {
logger.debug('status broadcast playerState=%s', status.playerState);
if (status.idleReason === 'FINISHED') {
await setVolume({ level });

Check warning on line 55 in server/services/google-cast/lib/google_cast.setValue.js

View check run for this annotation

Codecov / codecov/patch

server/services/google-cast/lib/google_cast.setValue.js#L55

Added line #L55 was not covered by tests
}
});

logger.debug('app "%s" launched, loading media %s ...', player.session.displayName, media.contentId);
Expand Down
5 changes: 3 additions & 2 deletions server/services/sonos/lib/sonos.setValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ const { DEVICE_FEATURE_TYPES } = require('../../../utils/constants');
* @param {object} device - Updated Gladys device.
* @param {object} deviceFeature - Updated Gladys device feature.
* @param {string|number} value - The new device feature value.
* @param {object} options - Optional configs.
* @example
* setValue(device, deviceFeature, 0);
*/
async function setValue(device, deviceFeature, value) {
async function setValue(device, deviceFeature, value, options) {
const deviceUuid = device.external_id.split(':')[1];
const sonosDevice = this.manager.Devices.find((d) => d.uuid === deviceUuid);
if (deviceFeature.type === DEVICE_FEATURE_TYPES.MUSIC.PLAY) {
Expand All @@ -30,7 +31,7 @@ async function setValue(device, deviceFeature, value) {
await sonosDevice.PlayNotification({
trackUri: value,
onlyWhenPlaying: false,
volume: 45, // Set the volume for the notification (and revert back afterwards)
volume: options?.volume || 45, // Set the volume for the notification (and revert back afterwards)
timeout: 20, // If the events don't work (to see when it stops playing) or if you turned on a stream,
// it will revert back after this amount of seconds.
delayMs: 700, // Pause between commands in ms, (when sonos fails to play sort notification sounds).
Expand Down
8 changes: 7 additions & 1 deletion server/test/services/airplay/lib/airplay.setValue.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,18 @@ describe('AirplayHandler.setValue', () => {
await airplayHandler.setValue(device, device.features[0], 'http://play-url.com');
sinon.assert.calledOnce(pipe);
});
it('should talk on speaker with custom volume', async () => {
airplayHandler.scanTimeout = 1;
const devices = await airplayHandler.scan();
const device = devices[0];
await airplayHandler.setValue(device, device.features[0], 'http://play-url.com', { volume: 30 });
});
it('should return device not found', async () => {
airplayHandler.scanTimeout = 1;
const device = {
external_id: 'airplay:toto',
};
const promise = airplayHandler.setValue(device, {}, 'http://play-url.com');
const promise = airplayHandler.setValue(device, {}, 'http://play-url.com', { volume: 30 });
await assert.isRejected(promise, 'Device not found on network');
});
});
14 changes: 12 additions & 2 deletions server/test/services/google-cast/lib/google_cast.setValue.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ class GoogleCastClient {
cb({ message: 'this is an error' });
}

// eslint-disable-next-line class-methods-use-this
getVolume(cb) {
cb(null, { level: 1 });
}

// eslint-disable-next-line class-methods-use-this
setVolume(volume, cb) {
cb(null, 30);
}

// eslint-disable-next-line class-methods-use-this
close() {}
}
Expand Down Expand Up @@ -80,14 +90,14 @@ describe('GoogleCastHandler.setValue', () => {
googleCastHandler.scanTimeout = 1;
const devices = await googleCastHandler.scan();
const device = devices[0];
await googleCastHandler.setValue(device, device.features[0], 'http://play-url.com');
await googleCastHandler.setValue(device, device.features[0], 'http://play-url.com', { volume: 30 });
});
it('should return device not found', async () => {
googleCastHandler.scanTimeout = 1;
const device = {
external_id: 'google_cast:toto',
};
const promise = googleCastHandler.setValue(device, {}, 'http://play-url.com');
const promise = googleCastHandler.setValue(device, {}, 'http://play-url.com', { volume: 30 });
await assert.isRejected(promise, 'Device not found on network');
});
});
27 changes: 27 additions & 0 deletions server/test/services/sonos/lib/sonos.setValue.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,4 +197,31 @@ describe('SonosHandler.setValue', () => {
delayMs: 700,
});
});
it('should play notification on Sonos and change volume', async () => {
const device = {
name: 'My sonos',
external_id: 'sonos:test-uuid',
service_id: 'ffa13430-df93-488a-9733-5c540e9558e0',
should_poll: false,
};
const deviceFeature = {
name: 'My sonos - Play notification',
external_id: 'sonos:test-uuid:play-notification',
category: 'music',
type: 'play_notification',
min: 1,
max: 1,
keep_history: false,
read_only: false,
has_feedback: false,
};
await sonosHandler.setValue(device, deviceFeature, 'http://test.com', { volume: 30 });
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you create a new test to keep the default volume test ?

assert.calledWith(devicePlayNotification, {
onlyWhenPlaying: false,
timeout: 20,
trackUri: 'http://test.com',
volume: 30,
delayMs: 700,
});
});
});
Loading