Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[hue] Fix assignment of serial number property #6094

Merged
merged 3 commits into from
Sep 18, 2019

Conversation

cweitkamp
Copy link
Contributor

@cweitkamp cweitkamp commented Sep 17, 2019

  • Fix assignment of serial number property - it is not always the same like the mac address (e.g. if you are using an emulated Hue API)
  • Added properties for mac address and model id

The serial number is the representational property of discovered devices. If we overwrite it with a wrong value the same device will be discovered again and again. The calculation of the serial number from the bridge id is derived from nUPnP discovery:

https://github.com/openhab/openhab2-addons/blob/a1d1c9cc3762ffee08c455a4d1928b1f82868a25/bundles/org.openhab.binding.hue/src/main/java/org/openhab/binding/hue/internal/discovery/HueBridgeNupnpDiscovery.java#L89

Signed-off-by: Christoph Weitkamp [email protected]

…like tht mac address

Signed-off-by: Christoph Weitkamp <[email protected]>
@cweitkamp cweitkamp added the bug An unexpected problem or unintended behavior of an add-on label Sep 17, 2019
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/bug-in-persistentinbox/82031/3

Signed-off-by: Christoph Weitkamp <[email protected]>
Signed-off-by: Christoph Weitkamp <[email protected]>
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kaikreuzer kaikreuzer merged commit e7a53e0 into openhab:master Sep 18, 2019
@cweitkamp cweitkamp deleted the bugfix-hue-discovery branch September 18, 2019 14:23
@wborn wborn added this to the 2.5 milestone Sep 29, 2019
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/hue-auto-discovery-finds-the-same-bridge-all-the-time/39689/5

@andrewfg
Copy link
Contributor

andrewfg commented Oct 13, 2019

Hi @cweitkamp, I think this issue is still not fixed: I am running the latest snapshot which I downloaded today, so I believe that the binding JAR file is indeed a build that does contain your changes (??) Anyway, with this version I am still getting lots of the following discovery messages every few minutes. Note that this relates to the discovery of my own Hue Bridge which is a) instantiated as a Thing via my .things file, and b) is nevertheless also being repeatedly discovered in the Inbox.

2019-10-13 15:30:50.816 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'hue:bridge:0017882157c7' to inbox.
2019-10-13 15:35:53.059 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'hue:bridge:0017882157c7' to inbox.
2019-10-13 15:37:41.809 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'hue:bridge:0017882157c7' to inbox.
2019-10-13 15:40:59.684 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'hue:bridge:0017882157c7' to inbox.

Note 1) it's just an idea, and may or may not be the cause of the problem, but please notice that there is different capitalisation of the hex 'C' in the following attributes..

  • the serialNumber is 0017882157C7 (capital "C")
  • the Mac Address is 00:17:88:21:57:c7 (lowercase "c")
  • the discovery log UID is hue:bridge:0017882157c7 (lowercase "c")

Note 2) when I click on the discovered bridge in the Inbox, and to try to instantiate it, then I get an error message "not found"..

2019-10-13 15:51:33.251 [ERROR] [ore.internal.discovery.InboxResource] - Thing hue:bridge:0017882157c7 unable to be approved: No Thing with UID hue:bridge:0017882157c7 in inbox

@andrewfg
Copy link
Contributor

PS you may be interested to see the following interleaved reports between openhab.log and events.log, which show that the bridge is in fact being repeatedly added and DELETED again from the Inbox..

**openhab.log**
2019-10-13 18:05:11.487 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'hue:bridge:0017882157c7' to inbox.

**events.log**
2019-10-13 18:05:11.488 [home.event.InboxAddedEvent] - Discovery Result with UID 'hue:bridge:0017882157c7' has been added.
2019-10-13 18:06:52.049 [me.event.InboxRemovedEvent] - Discovery Result with UID 'hue:bridge:0017882157c7' has been removed.

**openhab.log**
2019-10-13 18:07:14.479 [INFO ] [g.discovery.internal.PersistentInbox] - Added new thing 'hue:bridge:0017882157c7' to inbox.

**events.log**
2019-10-13 18:07:14.481 [home.event.InboxAddedEvent] - Discovery Result with UID 'hue:bridge:0017882157c7' has been added.
2019-10-13 18:08:55.107 [me.event.InboxRemovedEvent] - Discovery Result with UID 'hue:bridge:0017882157c7' has been removed.

@cweitkamp
Copy link
Contributor Author

@andrewfg There was a second bug in OHC causing duplicate inbox items (see openhab/openhab-core#1051). Now the interesting question is: Which OH version are you using?

@andrewfg
Copy link
Contributor

andrewfg commented Oct 13, 2019

@andrewfg There was a second bug in OHC causing duplicate inbox items (see openhab/openhab-core#1051). Now the interesting question is: Which OH version are you using?

I am using OH 2 version 2.5.0-SNAPSHOT whatever is the current snapshot for today October 13th (build #1721)

@andrewfg
Copy link
Contributor

andrewfg commented Oct 15, 2019

I have been doing a bit more snooping..

In the DiscoveryResult JSON database (org.eclipse.smarthome.config.discovery.DiscoveryResult.json) there is the following entry for the hue hub. => Note that it has four properties "ipAddress", "protocol", "serialNumber" and "port"..

 "hue:bridge:0017882157c7": {
    "class": "org.eclipse.smarthome.config.discovery.internal.DiscoveryResultImpl",
    "value": {
      "thingUID": {
        "segments": [
          "hue",
          "bridge",
          "0017882157c7"
        ]
      },
      "thingTypeUID": {
        "segments": [
          "hue",
          "bridge"
        ]
      },
      "properties": {
        "ipAddress": "192.168.1.108",
        "protocol": "http",
        "serialNumber": "0017882157c7",
        "port": 80
      },
      "representationProperty": "serialNumber",
      "flag": "NEW",
      "label": "Philips hue (192.168.1.108)",
      "timestamp": 1571148748411,
      "timeToLive": -1
    }
  },

In the UpnP discovery code (HueBridgeNupnpDiscovery.java) the DiscoveryResult creates two properties "ipAddress" and "serialNumber"; so I was wondering where the extra two properties are coming from..

String serialNumber = bridge.getId().substring(0, 6) + bridge.getId().substring(10);
ThingUID uid = new ThingUID(THING_TYPE_BRIDGE, serialNumber);
DiscoveryResult result = DiscoveryResultBuilder.create(uid)
    .withProperties(buildProperties(host, serialNumber))
    .withLabel(LABEL_PATTERN.replace("IP", host))
    .withRepresentationProperty(PROPERTY_SERIAL_NUMBER)
    .build();

Therefore I looked through the remaining hue code, and found another module (HueBridgeDiscoveryParticipant.java) that also produces a DiscoveryResult with four properties..

properties.put(HOST, device.getDetails().getBaseURL().getHost());
properties.put(PORT, device.getDetails().getBaseURL().getPort());
properties.put(PROTOCOL, device.getDetails().getBaseURL().getProtocol());
properties.put(PROPERTY_SERIAL_NUMBER, device.getDetails().getSerialNumber());

DiscoveryResult result = DiscoveryResultBuilder.create(uid)
    .withProperties(properties)
    .withLabel(device.getDetails().getFriendlyName())
    .withRepresentationProperty(PROPERTY_SERIAL_NUMBER)
    .build();

So I am wondering if the problem is perhaps due to some kind interference between these two different discovery processes ??

@andrewfg
Copy link
Contributor

andrewfg commented Oct 15, 2019

Yup, it very much seems that the binding is running two different discovery services in parallel

  1. Regular UPnP Discovery on http://192.168.1.xxx/description.xml
<root xmlns="urn:schemas-upnp-org:device-1-0">
<specVersion>
<major>1</major>
<minor>0</minor>
</specVersion>
<URLBase>http://192.168.1.xxx:80/</URLBase>
<device>
<deviceType>urn:schemas-upnp-org:device:Basic:1</deviceType>
<friendlyName>Philips hue (192.168.1.xxx)</friendlyName>
<manufacturer>Signify</manufacturer>
<manufacturerURL>http://www.meethue.com</manufacturerURL>
<modelDescription>Philips hue Personal Wireless Lighting</modelDescription>
<modelName>Philips hue bridge 2015</modelName>
<modelNumber>BSB002</modelNumber>
<modelURL>http://www.meethue.com</modelURL>
<serialNumber>0017882157c7</serialNumber>
<UDN>uuid:2f402f80-da50-11e1-9b23-0017882157c7</UDN>
<presentationURL>index.html</presentationURL>
<iconList>
<icon>
<mimetype>image/png</mimetype>
<height>48</height>
<width>48</width>
<depth>24</depth>
<url>hue_logo_0.png</url>
</icon>
</iconList>
</device>
</root>

  1. Philips private JSON discovery on https://www.meethue.com/api/nupnp

[{"id":"001788fffe2157c7","internalipaddress":"192.168.1.xxx"}]

thewiep pushed a commit to thewiep/openhab-addons that referenced this pull request Nov 23, 2019
Pshatsillo pushed a commit to Pshatsillo/openhab-addons that referenced this pull request Dec 8, 2019
tmrobert8 pushed a commit to tmrobert8/openhab-addons that referenced this pull request Jan 21, 2020
Signed-off-by: Christoph Weitkamp <[email protected]>
Signed-off-by: Tim Roberts <[email protected]>
hww3 pushed a commit to hww3/openhab2-addons that referenced this pull request Feb 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants