From 1e3725ad062c5211de429c047020db3bcf80e80c Mon Sep 17 00:00:00 2001 From: Cyril Beslay Date: Wed, 18 Nov 2020 19:43:05 +0100 Subject: [PATCH 1/3] fix: Answer message when no lights found in a room Closes #959 --- server/config/brain/light/answers.en.json | 4 ++++ server/config/brain/light/answers.fr.json | 4 ++++ server/lib/device/light/light.command.js | 4 ++++ server/test/lib/device/light/light.command.test.js | 11 +++++++++++ 4 files changed, 23 insertions(+) diff --git a/server/config/brain/light/answers.en.json b/server/config/brain/light/answers.en.json index 5d4a5bbab2..451f5a7625 100644 --- a/server/config/brain/light/answers.en.json +++ b/server/config/brain/light/answers.en.json @@ -14,5 +14,9 @@ { "label": "light.turn-off.fail", "answers": ["I didn't manage to turn off the light."] + }, + { + "label": "light.not-found", + "answers": ["I didn't find any light in this room."] } ] diff --git a/server/config/brain/light/answers.fr.json b/server/config/brain/light/answers.fr.json index 58070d4ef8..0ac8861506 100644 --- a/server/config/brain/light/answers.fr.json +++ b/server/config/brain/light/answers.fr.json @@ -14,5 +14,9 @@ { "label": "light.turn-off.fail", "answers": ["Je n'ai pas réussi à éteindre la lumière."] + }, + { + "label": "light.not-found", + "answers": ["Je n'ai pas trouvé de lumière dans cette pièce."] } ] diff --git a/server/lib/device/light/light.command.js b/server/lib/device/light/light.command.js index bd8a2bc331..853253f7b9 100644 --- a/server/lib/device/light/light.command.js +++ b/server/lib/device/light/light.command.js @@ -15,6 +15,10 @@ const { DEVICE_FEATURE_CATEGORIES, DEVICE_FEATURE_TYPES } = require('../../../ut async function command(message, classification, context) { try { const devices = await this.getLightsInRoom(context.room); + if (devices.length === 0) { + this.messageManager.replyByIntent(message, 'light.not-found', context); + return; + } switch (classification.intent) { case 'light.turn-on': // foreach devices in room diff --git a/server/test/lib/device/light/light.command.test.js b/server/test/lib/device/light/light.command.test.js index 5c29288756..efd931eb51 100644 --- a/server/test/lib/device/light/light.command.test.js +++ b/server/test/lib/device/light/light.command.test.js @@ -65,4 +65,15 @@ describe('Light.command', () => { assert.calledWith(messageManager.replyByIntent, message, 'light.turn-off.success', context); assert.called(testService.device.setValue); }); + it('should fail to send a command because no device in this room', async () => { + const stateManager = new StateManager(event); + const deviceManager = new Device(event, messageManager, stateManager, service); + // Mock getLightsInRoom to answer no devices + deviceManager.lightManager.getLightsInRoom = () => + new Promise((resolve) => { + resolve([]); + }); + await deviceManager.lightManager.command(message, { intent: 'light.turn-off' }, context); + assert.calledWith(messageManager.replyByIntent, message, 'light.not-found', context); + }); }); From bf044ff3ca470707f63fe2f68a04788d4dbecf61 Mon Sep 17 00:00:00 2001 From: Cyril Beslay Date: Thu, 19 Nov 2020 12:13:17 +0100 Subject: [PATCH 2/3] fix: Take in account PR feedbacks - Do not check number of devices but number of devices with binary features --- server/lib/device/light/light.command.js | 18 ++++++++++++------ .../lib/device/light/light.command.test.js | 12 ++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/server/lib/device/light/light.command.js b/server/lib/device/light/light.command.js index 853253f7b9..cb74df9efa 100644 --- a/server/lib/device/light/light.command.js +++ b/server/lib/device/light/light.command.js @@ -13,12 +13,9 @@ const { DEVICE_FEATURE_CATEGORIES, DEVICE_FEATURE_TYPES } = require('../../../ut * light.command(message, classification, context); */ async function command(message, classification, context) { + let nbLightsAffected = 0; try { const devices = await this.getLightsInRoom(context.room); - if (devices.length === 0) { - this.messageManager.replyByIntent(message, 'light.not-found', context); - return; - } switch (classification.intent) { case 'light.turn-on': // foreach devices in room @@ -32,9 +29,12 @@ async function command(message, classification, context) { // and if the binary feature exists, call it if (deviceFeature) { await this.turnOn(device, deviceFeature); + nbLightsAffected += 1; } }); - this.messageManager.replyByIntent(message, 'light.turn-on.success', context); + if (nbLightsAffected > 0) { + this.messageManager.replyByIntent(message, 'light.turn-on.success', context); + } break; case 'light.turn-off': // foreach devices in room @@ -48,9 +48,12 @@ async function command(message, classification, context) { // and if the binary feature exists, call it if (deviceFeature) { await this.turnOff(device, deviceFeature); + nbLightsAffected += 1; } }); - this.messageManager.replyByIntent(message, 'light.turn-off.success', context); + if (nbLightsAffected > 0) { + this.messageManager.replyByIntent(message, 'light.turn-off.success', context); + } break; default: throw new Error('Not found'); @@ -59,6 +62,9 @@ async function command(message, classification, context) { logger.debug(e); this.messageManager.replyByIntent(message, 'light.turn-on.fail', context); } + if (nbLightsAffected === 0) { + this.messageManager.replyByIntent(message, 'light.not-found', context); + } } module.exports = { diff --git a/server/test/lib/device/light/light.command.test.js b/server/test/lib/device/light/light.command.test.js index efd931eb51..5710fe0822 100644 --- a/server/test/lib/device/light/light.command.test.js +++ b/server/test/lib/device/light/light.command.test.js @@ -17,6 +17,12 @@ const testServiceBroken = { }, }; +const testServiceNoLight = { + getDeviceFeature: () => { + return null; + }, +}; + const service = { getService: () => testService, }; @@ -65,6 +71,12 @@ describe('Light.command', () => { assert.calledWith(messageManager.replyByIntent, message, 'light.turn-off.success', context); assert.called(testService.device.setValue); }); + it('should fail to send a command because no device with binary feature in this room', async () => { + const stateManager = new StateManager(event); + const deviceManager = new Device(event, messageManager, stateManager, testServiceNoLight); + await deviceManager.lightManager.command(message, { intent: 'light.turn-off' }, context); + assert.calledWith(messageManager.replyByIntent, message, 'light.not-found', context); + }); it('should fail to send a command because no device in this room', async () => { const stateManager = new StateManager(event); const deviceManager = new Device(event, messageManager, stateManager, service); From 8fcbbb79241152c3deb17d3ecd38034f119f2cd4 Mon Sep 17 00:00:00 2001 From: Cyril Beslay Date: Fri, 20 Nov 2020 18:55:28 +0100 Subject: [PATCH 3/3] fix(philips-hue): Use case where exception is thrown and no lights --- server/lib/device/light/light.command.js | 16 ++++++++-------- .../lib/device/light/light.command.test.js | 19 ++++++++++++------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/server/lib/device/light/light.command.js b/server/lib/device/light/light.command.js index cb74df9efa..9acf0d5abd 100644 --- a/server/lib/device/light/light.command.js +++ b/server/lib/device/light/light.command.js @@ -13,7 +13,7 @@ const { DEVICE_FEATURE_CATEGORIES, DEVICE_FEATURE_TYPES } = require('../../../ut * light.command(message, classification, context); */ async function command(message, classification, context) { - let nbLightsAffected = 0; + let nbAffectedLights = 0; try { const devices = await this.getLightsInRoom(context.room); switch (classification.intent) { @@ -29,10 +29,10 @@ async function command(message, classification, context) { // and if the binary feature exists, call it if (deviceFeature) { await this.turnOn(device, deviceFeature); - nbLightsAffected += 1; + nbAffectedLights += 1; } }); - if (nbLightsAffected > 0) { + if (nbAffectedLights > 0) { this.messageManager.replyByIntent(message, 'light.turn-on.success', context); } break; @@ -48,23 +48,23 @@ async function command(message, classification, context) { // and if the binary feature exists, call it if (deviceFeature) { await this.turnOff(device, deviceFeature); - nbLightsAffected += 1; + nbAffectedLights += 1; } }); - if (nbLightsAffected > 0) { + if (nbAffectedLights > 0) { this.messageManager.replyByIntent(message, 'light.turn-off.success', context); } break; default: throw new Error('Not found'); } + if (nbAffectedLights === 0) { + this.messageManager.replyByIntent(message, 'light.not-found', context); + } } catch (e) { logger.debug(e); this.messageManager.replyByIntent(message, 'light.turn-on.fail', context); } - if (nbLightsAffected === 0) { - this.messageManager.replyByIntent(message, 'light.not-found', context); - } } module.exports = { diff --git a/server/test/lib/device/light/light.command.test.js b/server/test/lib/device/light/light.command.test.js index 5710fe0822..fc46be4962 100644 --- a/server/test/lib/device/light/light.command.test.js +++ b/server/test/lib/device/light/light.command.test.js @@ -17,12 +17,6 @@ const testServiceBroken = { }, }; -const testServiceNoLight = { - getDeviceFeature: () => { - return null; - }, -}; - const service = { getService: () => testService, }; @@ -73,7 +67,18 @@ describe('Light.command', () => { }); it('should fail to send a command because no device with binary feature in this room', async () => { const stateManager = new StateManager(event); - const deviceManager = new Device(event, messageManager, stateManager, testServiceNoLight); + const deviceManager = new Device(event, messageManager, stateManager, service); + // Mock getDeviceFeature to answer no binay feature + deviceManager.lightManager.getLightsInRoom = () => + new Promise((resolve) => { + resolve([ + { + device: { + getDeviceFeature: () => null, + }, + }, + ]); + }); await deviceManager.lightManager.command(message, { intent: 'light.turn-off' }, context); assert.calledWith(messageManager.replyByIntent, message, 'light.not-found', context); });