Allow to use NEEO_SERVER_IP, NEEO_SERVER_BASEURL, NEEO_SERVER_PORT as Environment Variable

[splattner]

My suggestion for #170

This allows to set the full BASEURL via Environment Variable or just the ADAPTERIPADDRESS and then let the SDK build the baseurl.

[splattner]

I guess I should move part of this into the neeo-sdk-toolkit?

[splattner]

I also added ADAPTERPORT as Env Variable, so I can set this when running multiple Docker Container with NEEO SDK Devices

[splattner]

ok, I just figured out, that I probably should use the neeo-sdk-toolkit to run multiple Devices in one Container. But the Problem with the wrong IP Address taken for the baseURL still exists there. Therefore my 2 PRs:

• Allow to use NEEO_SERVER_IP, NEEO_SERVER_BASEURL, NEEO_SERVER_PORT as Environment Variable #171
• Allow to use NEEO_SERVER_IP or NEEO_SERVER_BASEURL as Environment Variable neeo-sdk-toolkit#5

[MichaelKohler]

Can you send us also the Dockerfile you are using so we can reproduce this? Also, how do you run Docker and on which OS?

[splattner]

Sure, check https://github.com/splattner/neeo-driver-googlecast

FROM node:latest

WORKDIR /app
COPY package.json /app
RUN npm install

COPY . /app
EXPOSE 6336

CMD ["npm","start"]

or by using the newer @neeo/cli

[...]
CMD ["npx","neeo-cli","start"]

I run Docker on my Ubuntu 18.04 and on my Synology NAS

docker build -t splattner/neeo-driver-googlecast .
docker run --rm --name neeo-driver-googlecast --network host -e "BRAINIP=192.168.10.211" -e "DEBUG=*" splattner/neeo-driver-googlecast

The Problem is generateBaseUrl in lib/device/index.js or better getAnyIpAddress in lib/device/validation/iphelper.js which does return the Containers eth0 Adress when run in Bridge configuration (which is obviously not reachable from the Brain) or returns the first non internal IPv4 Address when run in Host-Network Configuration (in my case this was vboxnet0 which is also not reachable from the Brain)

with my PR's you can use NEEO_SERVER_IP to override the getAnyIpAddress Function or use NEEO_SERVER_BASEURL to completly override the baseURL and don't use the generateBaseUrl Function.

[pfiaux]

Can you explain why the baseurl variable is needed here? it doesn't seem used below and the url is generated from the IP address.

[splattner]

in https://github.com/NEEOInc/neeo-sdk/blob/master/lib/device/index.js#L48 its only generated when conf.baseurl is not set. With this change, you can set conf.baseurl with an ENV

Generating the baseURL by IP Address might not always work (e.g. behind NAT), I even would suggest to not build at all a baseURL that needs to be send to the Brain. But thats an other Topic and probably a bit more tricky.

[pfiaux]

oh OK makes sense, I didn't see it in the PR and I was confused, my bad.

[splattner]

it's not in the PR :) it was already there. np 👍

[pfiaux]

I see the use case for this, that makes sense. As you mentioned if we go this way it's more important to add this in the neeo-sdk-toolkit/cli since it will replace this neeo-sdk cli (which will eventually be removed).

I'm wondering if we need both adapterIpAddress and baseurl, only 1 might be enough and will keep the interface more concise.

• the IP is easier to use (don't need to build URL), but for some edge cases having both is more flexible
• From the SDK perspective, they overlap adding complexity to the API, possible confusion over which to use when, and we'll have to maintain both (it's easier to add than remove without breaking changes)
• As a third alternative we might also be able to improve iphelper to cover the docker use case (and use neither)

The end goal for you is mainly to use the SDK within docker, correct?

[splattner]

I see the use case for this, that makes sense. As you mentioned if we go this way it's more important to add this in the neeo-sdk-toolkit/cli since it will replace this neeo-sdk cli (which will eventually be removed).

See NEEOInc/neeo-sdk-toolkit#5
I made the modifications there, but it requires some changes from this PR (the change in lib/device/index.js)
In the neeo-sdk-toolkit the ENV Handling is anyway nicer! Part of this PR is just for Backwards Compatibilty.

I'm wondering if we need both adapterIpAddress and baseurl, only 1 might be enough and will keep the interface more concise.

• the IP is easier to use (don't need to build URL), but for some edge cases having both is more flexible
• From the SDK perspective, they overlap adding complexity to the API, possible confusion over which to use when, and we'll have to maintain both (it's easier to add than remove without breaking changes)

I agree, but as the baseURL was already there as a SDKOption, I wanted to leave it as it is (and just add the Possibility to set it via ENV).
The Reason for adding the The NEEO_SERVER_IP is that the generateBaseURL uses the correct Port for building the URL, whereas if you use NEEO_SERVER_BASEURL you have to be aware of the correct Port.

• As a third alternative we might also be able to improve iphelper to cover the docker use case (and use neither)

The end goal for you is mainly to use the SDK within docker, correct?

Yes, at least for me it is, but also on Devices with multiple Network Interface (e.g. from a Viritualization Network, Virtualbox or similar)