Closed ZeroOne3010 closed 4 years ago
I think for this protocol this is fine, but for UPnP for example different bridges could present themselves at different times, a more asynchronous setup would be nice. It would be more logical of there would be a continuing (for as long as the discover process is going on) listening process with callbacks or implementable methods like onBridgeFound(Bridge bridge). (sorry of not completely clear, I will try to make something)
Sorry for ghosting, I made what I meant in a pull request. It is not very pretty and doesn't have any tests or comments yet. What do you think about it?
I found the N-UPnP method to be the most straightforward. I have it working alongside your library, I might expand it and create a pull request if I can find the time.
You need to get the JSON response from https://discovery.meethue.com/, supports multiple Bridges. Response is one JSON Array with one object for each Bridge. Note the precautions mentioned on Hue's webpage.
Should run on a background thread, I have used Asynctask below. I'm also using OkHttp3, but HttpsUrlConnection works fine as well. This code just returns the first Bridge in the JSON Array, it's pretty bare but I hope it helps guide people in the right direction.
// ...AsyncTask<Void, Void, String> {
//
@Override
protected String doInBackground(Void... voids) {
// Establish connection
OkHttpClient.Builder client = new OkHttpClient.Builder();
Request request = new Request.Builder()
.url("https://discovery.meethue.com/")
.build();
Response response = null;
String result = null;
try {
response = client.build().newCall(request).execute();
if (!response.isSuccessful()) {
return "";
} else {
result = response.body().string();
}
} catch (IOException e) {
e.printStackTrace();
}
// Parse JSON response
try {
JSONArray json = new JSONArray(result);
String bridgeIp, bridgeId;
for (int i = 0; i < json.length(); ++i) {
JSONObject bridge = json.getJSONObject(i);
bridgeId = bridge.getString("id");
bridgeIp = bridge.getString("internalipaddress");
Log.e(TAG, "Id: " + bridgeId + "\n" + "IP: " + bridgeIp);
return bridgeIp; // Return IP address of first Bridge
}
} catch (JSONException e) {
e.printStackTrace();
}
return "";
}
Fields are as follows, if at least one Bridge is found:
id: <string> (always present)
internalipaddress: <string> (always present)
macaddress: <string> (might be empty/null)
name: <string> (might be empty/null)
The problem is for the N-UPnP discoverer you need to have connection with internet, this is not always possible.
Thanks @JasperPluijmers and @bwalsh0! I pushed Jasper's code into a new branch called async-discoverer
and made a couple of minor edits. Development can continue in the branch. Some thoughts:
@ZeroOne3010, Didn't realize NUPnPDiscoverer was already implemented, will transition to it since your class is much more complete. Thank you for your work on this library.
OK, I think I've mostly completed the bullet points I outlined above, save for automatic unit tests or integration tests, as the interface has been changing so rapidly. Note to self from the documentation:
In case the application gets a non-empty response from the bridge, multiple bridges may have been found. Always validate information from these bridges before giving the user the option to select one bridge from the list for further control. There are two ways to check the connection with the bridge and retrieve some basic information from that bridge.
...so that still needs to be done.
I would still like to add mDNS discovery as well, but let's see if it'll be included in the first version or not. The IP scan seems like an overkill after all those other options, I don't really fancy doing that.
Whew... So, a few weeks ago I realized the UPnP discoverer wasn't working reliably at all and have been banging my head against the wall since. Today I finally figured it out: the SSDP message required quotation marks around one of the values..: https://github.com/ZeroOne3010/yetanotherhueapi/commit/cc1aea37f328ab16f8b1dfa1e68f30baf415284c :roll_eyes: In other words, progress is being made, but not as quickly as I would've liked.
Added an integration test for the UPnP discoverer and merged it all to master. :muscle: UPnP and NUPnP are now supported. I'm going to go ahead and close this issue.
Bridge discovery should be added. The Philips documentation is somewhat internally inconsistent, but the preferred order seems to be approximately like this:
Implement at least one of those.