pkmnct / homebridge-television-universal-control

This Homebridge plugin enables control of one or more compatible devices using one "Television" in HomeKit.
Apache License 2.0
9 stars 3 forks source link

Simplify UI for devices that only have a power state #12

Open asherkin opened 3 years ago

asherkin commented 3 years ago

Only add the TelevisionSpeaker service if we have a device with mute state implemented (which is required for TelevisionSpeaker), which simplifies the UI displayed in HomeKit for things that only have a power state.

Additionally, the empty array checks in the Homebridge callbacks weren't working as it turns out [] is truthy - which was causing Homebridge to complain that the callback was never called. This check shouldn't actually be needed any more with the first change, but I fixed that before realising the service could be removed completely.

I ran into this troubleshooting a warning from Homebridge:

[homebridge-television-universal-control] This plugin slows down Homebridge. The read handler for the characteristic 'Mute' didn't respond at all!. Please check that you properly call the callback! See https://git.io/JtMGR for more info.

with a very basic config for my projector:

{
  "groups": [
    {
      "name": "Projector",
      "devices": {
        "serial": [
          {
            "name": "Projector",
            "path": "/dev/ttyUSB0",
            "requestTimeout": 150,
            "delimiter": "\u0003",
            "getStatus": {
              "power": {
                "command": "\u0002QPW\u0003",
                "onResponse": "\u0002001",
                "offResponse": "\u0002000"
              }
            }
          }
        ]
      },
      "power": {
        "on": {
          "commands": [
            {
              "serial": [
                {
                  "commands": ["\u0002PON\u0003"],
                  "device": "Projector"
                }
              ]
            }
          ]
        },
        "off": {
          "commands": [
            {
              "serial": [
                {
                  "commands": ["\u0002POF\u0003"],
                  "device": "Projector"
                }
              ]
            }
          ]
        }
      },
      "inputs": [],
      "manufacturer": "Panasonic",
      "model": "AT6000E"
    }
  ],
  "platform": "TelevisionUniversalControl"
}
asherkin commented 1 year ago

@pkmnct it looks like you were prodding some of this in #17, any chance this could get reviewed / merged?

pkmnct commented 1 year ago

@pkmnct it looks like you were prodding some of this in #17, any chance this could get reviewed / merged?

Hey! Sorry for the delay, I commented some suggestions but this overall is looking sensible!

pkmnct commented 1 year ago

This fixes #8

asherkin commented 1 year ago

@pkmnct it looks like you were prodding some of this in #17, any chance this could get reviewed / merged?

Hey! Sorry for the delay, I commented some suggestions but this overall is looking sensible!

Heya - I can't see them I'm afraid!

If they're in a review, double-check the whole review was submitted at the top of the page 😄