Add `homebridge_media_player_switch` (on_off, play_pause, play_stop) #183

azogue · 2017-07-12T09:03:51Z

Introduction

The logic of behavior mode for the media_player switches is too rigid, and for many media player platforms, which implement multiple features (pause, turn_on/off, stop...), this makes theses switches not very useful, requiring extra switches to model the real needs.

Description

A new homebridge keyword homebridge_media_player_switch for customising the media players in HA is proposed, to specify the behaviour mode, between play_pause, on_off or play_stop options.

Also, the change state logic for media_player's is inverted. Instead of comparing with the on state, for media_players it should be 'different from the off state', because of the multiple 'active' options that normally a media player presents.

Example customize config in Home Assistant:

homeassistant:
  customize:
    media_player.kodi:
      homebridge_media_player_switch: on_off

This change fixes #171 and eliminates the need of extra variables.

… change state logic for media_player

mention-bot · 2017-07-12T09:03:52Z

@azogue, thanks for your PR! By analyzing the history of the files in this pull request, we identified @robbiet480, @maddox and @nunofgs to be potential reviewers.

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

  },
  getPowerState(callback) {
    this.log(`fetching power state for: ${this.name}`);

    this.client.fetchState(this.entity_id, (data) => {
      if (data) {
-        const powerState = data.state === this.onState;
+        const powerState = data.state != this.offState;


Expected '!==' and instead saw '!=' eqeqeq

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

@@ -56,14 +74,14 @@ function HomeAssistantMediaPlayer(log, data, client) {
 HomeAssistantMediaPlayer.prototype = {
  onEvent(oldState, newState) {
    this.switchService.getCharacteristic(Characteristic.On)
-        .setValue(newState.state === this.onState, null, 'internal');
+        .setValue(newState.state != this.offState, null, 'internal');


Expected '!==' and instead saw '!=' eqeqeq

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

-  } else if ((this.supportedFeatures | SUPPORT_TURN_ON) === this.supportedFeatures &&
-             (this.supportedFeatures | SUPPORT_TURN_OFF) === this.supportedFeatures) {
+  }
+  else if (support_on_off) {


Identifier 'support_on_off' is not in camel case camelcase

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

    this.onState = 'playing';
    this.offState = 'idle';
    this.onService = 'media_play';
    this.offService = 'media_stop';
-  } else if ((this.supportedFeatures | SUPPORT_TURN_ON) === this.supportedFeatures &&
-             (this.supportedFeatures | SUPPORT_TURN_OFF) === this.supportedFeatures) {
+  }


Closing curly brace does not appear on the same line as the subsequent block brace-style

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

    this.onState = 'playing';
    this.offState = 'paused';
    this.onService = 'media_play';
    this.offService = 'media_pause';
-  } else if ((this.supportedFeatures | SUPPORT_STOP) === this.supportedFeatures) {
+  }
+  else if (support_stop) {


Identifier 'support_stop' is not in camel case camelcase

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

+    this.offState = 'off';
+    this.onService = 'turn_on';
+    this.offService = 'turn_off';
+  }


Closing curly brace does not appear on the same line as the subsequent block brace-style

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

+  const support_on_off = ((this.supportedFeatures | SUPPORT_TURN_ON) === this.supportedFeatures &&
+                          (this.supportedFeatures | SUPPORT_TURN_OFF) === this.supportedFeatures);
+
+  if (this.data && this.data.attributes && this.data.attributes.homebridge_media_player_switch === 'on_off' && support_on_off) {


Identifier 'support_on_off' is not in camel case camelcase

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

-  if ((this.supportedFeatures | SUPPORT_PAUSE) === this.supportedFeatures) {
+  const support_pause = (this.supportedFeatures | SUPPORT_PAUSE) === this.supportedFeatures;
+  const support_stop = (this.supportedFeatures | SUPPORT_STOP) === this.supportedFeatures;
+  const support_on_off = ((this.supportedFeatures | SUPPORT_TURN_ON) === this.supportedFeatures &&


Identifier 'support_on_off' is not in camel case camelcase

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

@@ -31,18 +31,36 @@ function HomeAssistantMediaPlayer(log, data, client) {
    this.name = data.entity_id.split('.').pop().replace(/_/g, ' ');
  }

-  if ((this.supportedFeatures | SUPPORT_PAUSE) === this.supportedFeatures) {
+  const support_pause = (this.supportedFeatures | SUPPORT_PAUSE) === this.supportedFeatures;
+  const support_stop = (this.supportedFeatures | SUPPORT_STOP) === this.supportedFeatures;


Identifier 'support_stop' is not in camel case camelcase

houndci-bot · 2017-07-12T09:04:00Z

accessories/media_player.js

@@ -31,18 +31,36 @@ function HomeAssistantMediaPlayer(log, data, client) {
    this.name = data.entity_id.split('.').pop().replace(/_/g, ' ');
  }

-  if ((this.supportedFeatures | SUPPORT_PAUSE) === this.supportedFeatures) {
+  const support_pause = (this.supportedFeatures | SUPPORT_PAUSE) === this.supportedFeatures;


Identifier 'support_pause' is not in camel case camelcase

robbiet480 · 2017-07-19T10:34:32Z

Thanks! 🐬 🍪 💯

Add homebridge_media_player_switch (on_off, play_pause, play_stop),…

cf9af1f

… change state logic for media_player

homeassistant added the cla-signed label Jul 12, 2017

houndci-bot reviewed Jul 12, 2017

View reviewed changes

js style

bfa5718

azogue mentioned this pull request Jul 12, 2017

Define the turn on/off action for media player Kodi with scripts home-assistant/core#8408

Closed

2 tasks

Merge branch 'master' into kodi-player

44a6820

robbiet480 merged commit ea7a15a into home-assistant:master Jul 19, 2017

robbiet480 mentioned this pull request Jul 19, 2017

Media Player(s) from HomeAssistant do not show-up in HomeBridge as light switch #154

Closed

azogue mentioned this pull request Jul 28, 2017

SONOS media_player displays incorrect state #193

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `homebridge_media_player_switch` (on_off, play_pause, play_stop) #183

Add `homebridge_media_player_switch` (on_off, play_pause, play_stop) #183

azogue commented Jul 12, 2017 •

edited

Loading

mention-bot commented Jul 12, 2017

houndci-bot Jul 12, 2017

houndci-bot Jul 12, 2017

houndci-bot Jul 12, 2017

houndci-bot Jul 12, 2017

houndci-bot Jul 12, 2017

houndci-bot Jul 12, 2017

houndci-bot Jul 12, 2017

houndci-bot Jul 12, 2017

houndci-bot Jul 12, 2017

houndci-bot Jul 12, 2017

robbiet480 commented Jul 19, 2017

Add homebridge_media_player_switch (on_off, play_pause, play_stop) #183

Add homebridge_media_player_switch (on_off, play_pause, play_stop) #183

Conversation

azogue commented Jul 12, 2017 • edited Loading

Introduction

Description

Example customize config in Home Assistant:

mention-bot commented Jul 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robbiet480 commented Jul 19, 2017

Add `homebridge_media_player_switch` (on_off, play_pause, play_stop) #183

Add `homebridge_media_player_switch` (on_off, play_pause, play_stop) #183

azogue commented Jul 12, 2017 •

edited

Loading