ToddGreenfield / homebridge-directv

Homebridge plugin for Directv
1 stars 5 forks source link

Older DirecTV STBs with IP interfaces should work, and there can be multiple STBs, each with its own IP. At least here, they do not. I provide details. #17

Open Numbski opened 3 years ago

Numbski commented 3 years ago

So my equipment is old as dirt. I'll own up to that - but the json API is the same to my eye. Here's the output of my living room STB:

pi@homebridge:~/scripts/directv$ curl http://172.16.30.79:8080/info/getOptions | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  5873  100  5873    0     0  20607      0 --:--:-- --:--:-- --:--:-- 20679
{
  "options": [
    {
      "command": "/info/getLocations",
      "description": "List of available client locations. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        }
      ]
    },
    {
      "command": "/info/getSerialNum",
      "description": "STB serial number. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "clientAddr",
          "type": "string"
        }
      ]
    },
    {
      "command": "/info/getVersion",
      "description": "Set-top-box and SHEF information. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        }
      ]
    },
    {
      "command": "/info/mode",
      "description": "Set-top-box mode. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "clientAddr",
          "type": "string"
        }
      ]
    },
    {
      "command": "/remote/processKey",
      "description": "Process a key request from the remote control. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "clientAddr",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "hold",
          "type": "string"
        },
        {
          "isRequired": true,
          "name": "key",
          "type": "string"
        }
      ]
    },
    {
      "command": "/serial/processCommand",
      "description": "Process a command request from remote control. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        },
        {
          "isRequired": true,
          "name": "cmd",
          "type": "hex"
        }
      ]
    },
    {
      "command": "/tv/getProgInfo",
      "description": "Program information of specified channel at current or specific time. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "minor",
          "type": "int"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        },
        {
          "isRequired": true,
          "name": "major",
          "type": "int"
        },
        {
          "isRequired": false,
          "name": "clientAddr",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "time",
          "type": "long"
        }
      ]
    },
    {
      "command": "/tv/getTuned",
      "description": "Information about the currently viewed program. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "videoWindow",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "clientAddr",
          "type": "string"
        }
      ]
    },
    {
      "command": "/tv/tune",
      "description": "Tune to a channel. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "minor",
          "type": "int"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        },
        {
          "isRequired": true,
          "name": "major",
          "type": "int"
        },
        {
          "isRequired": false,
          "name": "source",
          "type": "int"
        },
        {
          "isRequired": false,
          "name": "clientAddr",
          "type": "string"
        }
      ]
    }
  ],
  "status": {
    "code": 200,
    "commandResult": 0,
    "msg": "OK.",
    "query": "/info/getOptions"
  }
}

Each of my 4 (yes, 4) STBs has their own IP address and respond almost identically. They're listening on port 8080 (from what I can tell, this is what the directv-remote module defaults to as well). I've tried setting with and without geni, and I've only been trying to get one working, as I am about 99% sure you didn't count on a situation with multiple STBs with their own IP addresses.

Just the same, here's the config I have in homebridge:

        {
            "platform": "Directv",
            "name": "DTV",
            "ip_address": "172.16.30.79",
            "exclude_geni": false,
            "min_channel": 1,
            "max_channel": 575
        }

Relaunch homebridge, and it will cause this error (tried with and without specifying port number, same results):

[10/19/2020, 10:59:35] Plugin 'homebridge-directv' tried to register with an incorrect plugin identifier: 'homebridge-directv-location'. Please report this to the developer!
[10/19/2020, 10:57:11] [DTV] Initializing Directv platform...
[10/19/2020, 10:57:11] [DTV] Fetching DTV locations.
10/19/2020, 10:57:11] Loading 10 accessories...
[10/19/2020, 10:58:39] [DTV] Finding DTV Locations (accessories)...
[10/19/2020, 10:58:39] TypeError: Cannot read property 'length' of undefined
    at /usr/local/lib/node_modules/homebridge-directv/index.js:48:40
    at /usr/local/lib/node_modules/homebridge-directv/node_modules/directv-remote/index.js:264:21
    at IncomingMessage.<anonymous> (/usr/local/lib/node_modules/homebridge-directv/node_modules/directv-remote/index.js:311:17)
    at IncomingMessage.emit (events.js:326:22)
    at endReadableNT (_stream_readable.js:1223:12)
    at processTicksAndRejections (internal/process/task_queues.js:84:21)
[10/19/2020, 11:01:20] [DTV] Failed to find any DTV Locations (accessories) - Check IP address in config.json!
[10/19/2020, 10:58:39] Got SIGTERM, shutting down Homebridge...

Ignore the timestamps. This is a patchwork copy and paste out of my logs. There's better ways to do it, but I need more coffee.

What appears to be happening is that the directv-remote module is attempting to pull this:

pi@homebridge:~/scripts/directv$ curl http://172.16.30.79:8080/info/getLocations | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   195  100   195    0     0   1772      0 --:--:-- --:--:-- --:--:--  1772
{
  "locations": [
    {
      "clientAddr": "0",
      "locationName": "LIVING ROOM"
    }
  ],
  "status": {
    "code": 200,
    "commandResult": 0,
    "msg": "OK.",
    "query": "/info/getLocations"
  }
}

https://github.com/cmrust/directv-remote/blob/master/index.js

It's at the very top of the file. The module validates the IP address by pulling exactly what I just pulled, and clearly it pulls valid json back and so far as I can tell, it should be found. I'm completely baffled. I was hoping if I opened an issue with this, your familiarity with how this thing ticks might ring some bells as to what could be so far off here as to break things outright. My gut says that it is failing due to using the default port of 80 instead of 8080, but that doesn't immediately add up, because if you look at the very top of the directv-remote module, you have this bit:

    var options = {
        hostname: IP_ADDRESS,
        port: 8080,
        path: '/info/getLocations'
    };

I'm at a point now where I'm likely to run a tcpdump and see if I can capture precisely what is getting sent to my STB, but this is just feels ridiculous. Got any suggestions?

Numbski commented 3 years ago
    172.16.30.28.54386 > 172.16.30.76.http-alt: Flags [P.], cksum 0x9505 (incorrect -> 0x27e4), seq 1:87, ack 1, win 502, options [nop,nop,TS val 1843789388 ecr 516813839], length 86: HTTP, length: 86
    GET /info/getLocations?type=1 HTTP/1.1
    Host: 172.16.30.76:8080
    Connection: close

?type=1 is different from what I was sending, so...

pi@homebridge:~/scripts/directv$ curl http://172.16.30.79:8080/info/getLocations?type=1
{"status": {
  "code": 403,
  "commandResult": 1,
  "msg": "Forbidden.Invalid URL parameter(s) found.",
  "query": "/info/getLocations?type=1"

...and there's the problem. That type=1 is breaking things here. Which would be helpful to know what "type=1" even is. Gah, internet obscura. I tell ya...

Numbski commented 3 years ago

Line 74 of directv-remote:

    // "List of available client locations."
    // Returns an array of the networked set top boxes
    // type is an optional parameter
    //  the docs label it as 'int'
    //  only 0 and 1 aren't *Forbidden*
    //  i'm not sure what the difference is yet,
    //  but 1 shows more of my wireless Genie STBs

OFFS. So it's optional, but the author threw it in not knowing what it did, and if you're on an older STB, that breaks things.

    this.getLocations = function(type, callback){
        var path = '/info/getLocations';

        var options = {
            hostname: this.IP_ADDRESS,
            port: 8080,
            path: path
        };

        if (typeof type !== 'undefined') {
            options.path = buildQueryString(options.path, { type: type });
        }

        makeRequest(options, callbackHandler(callback));
    };

So my primary language is perl, with some javascript and SQL circa Y2K. NodeJS is reeeeeaaaaalllly a stretch of my skillset right now, but if I am reading this correctly, one could safely reduce (options.path, { type: type }) down to just (options.path) to fix my particular scenario, but then that's altering the upstream codebase to satisfy a single antiquated user. I'll poke around in your code, see if you left an obvious place where I can alter things and re-test without causing breakage to you or anyone else using this, but that type argument is going to be frustrating, especially if the upstream author doesn't even know what it does.

Numbski commented 3 years ago

Line 45 of your module:

remote.getLocations('1', function(err, response) {

vs Line 90 of upstream:

        if (typeof type !== 'undefined') {
            options.path = buildQueryString(options.path, { type: type });
        }

        makeRequest(options, callbackHandler(callback));

So let me get this straight. type is "optional", but upstream has explicit checking to make sure thet type isn't undefined (which coincidentally is exactly what I want!), if it isn't undefined, we'll makeRequest(), otherwise we do nothing...

That sounds awfully "required" for an "optional" parameter. :\

Numbski commented 3 years ago

For full circular transparency:

https://github.com/cmrust/directv-remote/issues/2

Numbski commented 3 years ago

So I changed one line in each this module and upstream. On the upstream, I have it build the url with and without the type parameter, and with this module I brute forced it, because of a theory I have about this parameter to begin with:

Line 45

remote.getLocations(undefined, function(err, response) {

...and now it is finding a different STB than the one I was searching for.

[10/19/2020, 12:05:51] [DTV] Finding DTV Locations (accessories)...
[10/19/2020, 12:05:51] [DTV] Found DTV Location FAMILY ROOM
[10/19/2020, 12:05:51] Initializing platform accessory 'Family Room DTV'...

I'm just about out of time to spend on this for today, but I think I have demonstrated what should happen. My theory is that STBs pre-genie didn't have a type parameter at all. Post genie type=0 would be a standalone STB, and type=1 would be a genie with children. If not that, then type=0 might be a plain STB, and type=1 a DVR. That doesn't appear to apply over here though.

Will try to poke it a bit more with a stick. Clearly we need to make "type" truly optional in both places. I'm open to suggestions as to how to change this call to fix things, but forcing it undefined is probably not ideal.

Numbski commented 3 years ago

Correction about multiple platforms not working - I am wrong. With my "fix", all 4 seem to work. Trying to put them through their paces now. Would reeealy help to know with certainty what type actually is.

Numbski commented 3 years ago

Well, I got sucked in again. Upstream has a new pull request to fix their code. Things are working...sorta.

Except something is off. I've gone through the entire restful api to make sure arguments line up, and they very much do. Getting some interesting log spam though:

The read handler for the characteristic 'Channel' didn't respond at all!. Please check that you properly call the callback!
The read handler for the characteristic 'Channel' didn't respond at all!. Please check that you properly call the callback!
[Channel] characteristic was supplied illegal value: number 0 exceeded minimum of 1.
[Channel] characteristic was supplied illegal value: number 0 exceeded minimum of 1.
[Channel] characteristic was supplied illegal value: number 0 exceeded minimum of 1.

Starting to get out of my depth, but obviously 0 doesn't exceed 1, so...wtf. I have tried to read some documentation on plugin development, but I cannot possibly absorb enough, fast enough, to sort this out I think.

I have a basic approach laid out to be able to specify legacy STBs, but I would like to nail this down before committing it and putting a pull request in.

This is all very much territory I'm feeling out as I go.

Numbski commented 3 years ago

HAH! Correction!

Sure, all of the set top boxes appear in the UI, but none of them report the correct serial number...and all of the entries in Home/Eve control just one set top box. ???

So I'm wrong. It doesn't work with my change to handle them all. Somehow they aren't getting properly instantiated. Bleh.

They all control the one in the master bedroom. Conventiently, that's the last one in my config. So it must be an enumeration thing:

        {
            "platform": "Directv",
            "name": "FamilyRoomDTV",
            "ip_address": "172.16.30.76",
            "exclude_geni": true,
            "min_channel": 1,
            "max_channel": 575
        },
        {
            "platform": "Directv",
            "name": "LivingRoomDTV",
            "ip_address": "172.16.30.79",
            "exclude_geni": true,
            "min_channel": 1,
            "max_channel": 575
        },
        {
            "platform": "Directv",
            "name": "GarageDTV",
            "ip_address": "172.16.30.53",
            "exclude_geni": true,
            "min_channel": 1,
            "max_channel": 575
        },
        {
            "platform": "Directv",
            "name": "MasterBedroomDTV",
            "ip_address": "172.16.30.60",
            "exclude_geni": true,
            "min_channel": 1,
            "max_channel": 575
        }
Numbski commented 3 years ago

Going to stop and apologize for all of the spam you've been getting today because of me. I'm trying to contribute, and I'm pretty verbose when I get into this kind of stuff. It's not demanding of you, it has everything to do with documenting my experience.

[10/19/2020, 15:29:59] [MasterBedroomDTV] Unable to call for current channel at DTV location Master Bedroom.
[Channel] characteristic was supplied illegal value: number 0 exceeded minimum of 1.
[10/19/2020, 15:29:59] [FamilyRoomDTV] Unable to call for current channel at DTV location Family Room.
[Channel] characteristic was supplied illegal value: number 0 exceeded minimum of 1.

That's still flipping weird. I'm thinking of adding a logging entry that states what IP is getting sent to (although I already know the answer, I can't tell why).

Numbski commented 3 years ago

I think I see it now. You can't duplicate a platform with identical names, so either there would need to be a separate definition for each "platform", despite there only ever being exactly one address (0) for each, OR - do it what I perceive to be the "right way", and create an array of STBs that have actual IP addresses, each with my "compatibility" variable that determines whether it is genie era or legacy era (therefore determining type=0,1, or undefined), it's own IP address, it's own...well everything, per "accessory". That complicates matters a touch though, because we're also spawning accessories for child genie units.

If I could just wrap my brain around how to instantiate this correctly this could work though. Basically going from this:

        {
            "platform": "Directv",
            "name": "FamilyRoomDTV",
            "ip_address": "172.16.30.76",
            "compatibility": "legacy",
            "exclude_geni": true,
            "min_channel": 1,
            "max_channel": 575
        },
        {
            "platform": "Directv",
            "name": "LivingRoomDTV",
            "ip_address": "172.16.30.79",
            "compatibility": "legacy",
            "exclude_geni": true,
            "min_channel": 1,
            "max_channel": 575
        },
        {
            "platform": "Directv",
            "name": "GarageDTV",
            "ip_address": "172.16.30.53",
            "compatibility": "legacy",
            "exclude_geni": true,
            "min_channel": 1,
            "max_channel": 575
        },
        {
            "platform": "Directv",
            "name": "MasterBedroomDTV",
            "ip_address": "172.16.30.60",
            "compatibility": "legacy",
            "exclude_geni": true,
            "min_channel": 1,
            "max_channel": 575
        }

To this:

{
  "platform": "Directv",
  "stbs": [
    {
      "name": "FamilyRoomDTV",
      "ip_address": "172.16.30.76",
      "compatibility": "legacy",
      "exclude_geni": true,
      "min_channel": 1,
      "max_channel": 575
    },
    {
      "name": "LivingRoomDTV",
      "ip_address": "172.16.30.79",
      "compatibility": "legacy",
      "exclude_geni": true,
      "min_channel": 1,
      "max_channel": 575
    },
    {
      "name": "GarageDTV",
      "ip_address": "172.16.30.53",
      "compatibility": "legacy",
      "exclude_geni": true,
      "min_channel": 1,
      "max_channel": 575
    },
    {
      "name": "MasterBedroomDTV",
      "ip_address": "172.16.30.60",
      "compatibility": "legacy",
      "exclude_geni": true,
      "min_channel": 1,
      "max_channel": 575
    }
  ]
}
Numbski commented 3 years ago

I've committed my changes to the fork on my page. It isn't fully functional - as I said, it definitely pulls them all in, but they all use whichever dtv platform block was listed last in config.json, rather than the correct individual one (unless you happen to be the last in config.json, of course).

Numbski commented 3 years ago

Just in case the author happens to see any of this - it now occurs to me (hours later), that rather than my little "compatibility" flag, we could just test for .options[].urlParameters[].name == "type" existing and whether .options[].urlParameters[].isRequired is true or false where .options[].command=="/info/getLocations" (surely there's a better way of selecting that in one pass, and I'm just horrible with json!)

Basically, if .options[].urlParameters[].name == "type" exists in the response to /info/getLocations, then sent the type variable across. Otherwise send undefined.

Either way, take the option out of the hands of the end-user, force it to work. Still don't know how to handle the instantiation piece quite right.

...and this day is just gone. I shouldn't do this to myself.

{
  "options": [
    {
      "command": "/info/getLocations",
      "description": "List of available client locations. Warning: This command may change or be disabled in the future.",
      "formControls": [],
      "urlParameters": [
        {
          "isRequired": false,
          "name": "wrapper",
          "type": "string"
        },
        {
          "isRequired": false,
          "name": "callback",
          "type": "string"
        }
      ]
    }
Numbski commented 3 years ago

jq '.options[] | select(.command=="/info/getLocations") | select(.urlParameters[].name=="type")' getOptions.json

Basically if that filter returns null, send undefined. Not sure how this is accomplished in node, whether the filter is useful or not, but I tried. Lots and lots of trying today.

apexad commented 3 years ago

Did you get this working? Feel free to make a pull request with your changes.

Numbski commented 3 years ago

Hi Alex,

I got it working, and there is a pull request already for directv-remote, and that needs to be accepted before adding a pull request here, otherwise it will still be broken. I have forked directv-remote, added the fix, and put a pull request in.

Here, I had put a variable in that allows you to select legacy hardware, but I am rewriting it to auto detect. I am using that early version now, and so far it appears to work perfectly.

Tony Shadwick