[androidtv] Fix discovery issues

[morph166955]

Resolves issue with discovered items not being ignored correctly

Resolves #16223

[morph166955]

@jlaur Pulled the discovery fixes to here. This won't compile properly until I rebase after you merge the other since we changed PROPERTY_ to PARAMETER_.

[morph166955]

Rebased after the merge. This should just be the discovery fixes now.

[jlaur]

This PR was extracted from #16191 while this issue was still open: #16191 (comment)

[morph166955]

So what do you think is the issue @jlaur?

[jlaur]

So what do you think is the issue @jlaur?

I don't know. But for me to get the expected results, I only needed to fix this line (relative to main):

openhab-addons/bundles/org.openhab.binding.androidtv/src/main/java/org/openhab/binding/androidtv/internal/discovery/GoogleTVDiscoveryParticipant.java

Line 84 in d5bbda6

return DiscoveryResultBuilder.create(uid).withProperties(properties).withLabel(label).build();

into:

    return DiscoveryResultBuilder.create(uid).withProperties(properties)
        .withRepresentationProperty(PARAMETER_IP_ADDRESS).withLabel(label).build();

[morph166955]

That's so interesting to me. That line was pulled from the Roku binding to fix this. The odd part was that Roku had that in place when I was having the issue. Just to confirm, the host property didn't get stuck in the thing database when you tested it without that line correct?

[jlaur]

Just to confirm, the host property didn't get stuck in the thing database when you tested it without that line correct?

No, I tested on my 4.2 development system with a clean slate, i.e. no existing androidtv things. I then created the androidtv.things file and triggered the discovery. Before that I had nothing in my inbox, and after that I had the ignored entry.

[morph166955]

Very interesting. I'll run some more tests here.

[jlaur]

I can also add that my discovery result looks like this in userdata/jsondb/org.openhab.core.config.discovery.DiscoveryResult.json:

  "androidtv:googletv:DCE55BD7C4F7": {
    "class": "org.openhab.core.config.discovery.internal.DiscoveryResultImpl",
    "value": {
      "thingUID": {
        "segments": [
          "androidtv",
          "googletv",
          "DCE55BD7C4F7"
        ],
        "uid": "androidtv:googletv:DCE55BD7C4F7"
      },
      "thingTypeUID": {
        "segments": [
          "androidtv",
          "googletv"
        ],
        "uid": ""
      },
      "properties": {
        "ipAddress": "192.168.0.72"
      },
      "representationProperty": "ipAddress",
      "flag": "IGNORED",
      "label": "Kontor TV (DCE55BD7C4F7)",
      "timestamp": "2024-01-12T22:25:57.098186200Z",
      "timeToLive": -1
    }
  },

Perhaps it's worth comparing.

[morph166955]

@jlaur I am now baffled. I also now see that entry ignored without the addition of the property in the handler. It's unclear to me what's happening here. It's also unclear to me why Roku was also not working previously (I haven't tested it without that line again). Testing before was very conclusive that it needed that line, and now it's working just fine without it. I'll pull the line out of the handler for now.

[morph166955]

Also to note, I tried to return only thing thing uid for shield/google based on the comment here #16191 (comment) and that didn't resolve that issue. I'm not sure if it's possible to correlate across different thing types.

[jlaur]

Also to note, I tried to return only thing thing uid for shield/google based on the comment here #16191 (comment) and that didn't resolve that issue. I'm not sure if it's possible to correlate across different thing types.

Still, I think it should be fixed, I don't think it makes sense for both discovery services to support both thing types? At least it seems like it could cause some issues similar to the one you mentioned.

[jlaur]

Perhaps you could also simplify null annotation, now that you are at it. While reading both discovery classes the other day I noticed:

openhab-addons/bundles/org.openhab.binding.androidtv/src/main/java/org/openhab/binding/androidtv/internal/discovery/GoogleTVDiscoveryParticipant.java

Line 57 in afb20ca

public @Nullable DiscoveryResult createResult(@Nullable ServiceInfo service) {

should be:

public @Nullable DiscoveryResult createResult(ServiceInfo service) {

and:

openhab-addons/bundles/org.openhab.binding.androidtv/src/main/java/org/openhab/binding/androidtv/internal/discovery/GoogleTVDiscoveryParticipant.java

Line 94 in afb20ca

public @Nullable ThingUID getThingUID(@Nullable ServiceInfo service) {

should be:

public @Nullable ThingUID getThingUID(ServiceInfo service) {

same for both classes.

[morph166955]

Easy enough, standby I'll push that now.

[morph166955]

Perhaps you could also simplify null annotation, now that you are at it. While reading both discovery classes the other day I noticed:

openhab-addons/bundles/org.openhab.binding.androidtv/src/main/java/org/openhab/binding/androidtv/internal/discovery/GoogleTVDiscoveryParticipant.java

Line 57 in afb20ca

public @Nullable DiscoveryResult createResult(@Nullable ServiceInfo service) {

should be:

public @Nullable DiscoveryResult createResult(ServiceInfo service) {

and:

openhab-addons/bundles/org.openhab.binding.androidtv/src/main/java/org/openhab/binding/androidtv/internal/discovery/GoogleTVDiscoveryParticipant.java

Line 94 in afb20ca

public @Nullable ThingUID getThingUID(@Nullable ServiceInfo service) {

should be:

public @Nullable ThingUID getThingUID(ServiceInfo service) {

same for both classes.

That causes this...

[WARNING] /raid/openhab-dev/androidtv-16223/openhab-addons/bundles/org.openhab.binding.androidtv/src/main/java/org/openhab/binding/androidtv/internal/discovery/GoogleTVDiscoveryParticipant.java:[57,14] Redundant null check: comparing '@nonnull ServiceInfo' against null
[WARNING] /raid/openhab-dev/androidtv-16223/openhab-addons/bundles/org.openhab.binding.androidtv/src/main/java/org/openhab/binding/androidtv/internal/discovery/GoogleTVDiscoveryParticipant.java:[94,14] Redundant null check: comparing '@nonnull ServiceInfo' against null
[WARNING] /raid/openhab-dev/androidtv-16223/openhab-addons/bundles/org.openhab.binding.androidtv/src/main/java/org/openhab/binding/androidtv/internal/discovery/ShieldTVDiscoveryParticipant.java:[57,13] Redundant null check: comparing '@nonnull ServiceInfo' against null
[WARNING] /raid/openhab-dev/androidtv-16223/openhab-addons/bundles/org.openhab.binding.androidtv/src/main/java/org/openhab/binding/androidtv/internal/discovery/ShieldTVDiscoveryParticipant.java:[96,13] Redundant null check: comparing '@nonnull ServiceInfo' against null

Would it be cleaner to leave the Nullable in or to remove the null check after?

[jlaur]

Would it be cleaner to leave the Nullable in or to remove the null check after?

You can remove those null checks, that's why it's a simplification. 🙂

[jlaur]

Thanks!