[hdpowerview] Corrections to shade database and capabilities

[andrewfg]

In the Home Assistant integration see Issue #16, the developers, (supported by many other kind users), have assembled an extended collection of shade types, and tested the actual capabilities of existing and newly discovered shade types => @kingy444 many thanks for this great effort :)

In this PR I have updated the OpenHAB binding to reflect the learnings from the Home Assistant team.

Specific changes are..

• Capabilities 0: Removed tilt support
• Capabilities 3: Changed from tiltOnClosed() to tiltAnywhere()
• Capabilities 4: Removed tilt support
• Capabilities 9: Changed from tiltAnywhere() to tiltOnClosed()
• Capabilities 8 & 9: Fixed bugs in the position getting and setting functions
• Shade type 1: Added to database
• Shade type 18: Model name corrected
• Shade type 44: Added shade specific tiltOnClosed() support to override the capabilities 0 change above
• Shade types 69 .. 71: Changed capabilities from 3 to 4

Furthermore, I have modified the JUnit tests to test the Shade Position interlocking in accordance with the above functionality changes.

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

[jlaur]

Thanks! I'll have a closer look tomorrow and also verify if tiltOnClosed() removal for capabilities 0 has any impact on my Twist shades.

[andrewfg]

verify if tiltOnClosed() removal for capabilities 0 has any impact on my Twist shades.

I am pretty sure it does not. My own shades are Twist..

[jlaur]

Still some doubts, sorry.

[andrewfg]

I will get back tomorrow..

[jlaur]

Btw, created new version of documentation based on your changes:
/~https://github.com/jlaur/hdpowerview-doc/releases/tag/v1.0.4

[andrewfg]

I think the properties should only show what we actually received live in the shade data. So no "derived" or "overriden" properties. So we might need to modify or rename the properties and/or contents. Let me think about this..

[andrewfg]

So for example a shade would only report a property "tilt detected" if a JSON packet actually contained a posKind 3 entry. And such property might only be actualised only after the user had actually successfully executed a tilt command. Which may be never, if the user never issues such command. Hmm..

[andrewfg]

@jlaur upon reflection overnight (I woke up at 4 AM) I have made some changes (committed just now)..

1. Adopted your suggestion to move the functionality of overridden capabilities from the handler to the second getCapabilities() method (see code below).
2. Refactored the field name typeCapabilities to capabilitiesOverride to make it clearer what is going on.
3. Improved the Java Doc for the two getCapabilities() methods to make it clearer what is going on.

    /**
     * Return a Capabilities class instance that corresponds to the given 'capabilitiesId' parameter. If the
     * 'capabilitiesId' parameter is for a valid capabilities entry in the database, then that respective Capabilities
     * class instance is returned. Otherwise a blank Capabilities class instance is returned.
     *
     * @param capabilitiesId the target capabilities Id.
     * @return corresponding Capabilities class instance.
     */
    public Capabilities getCapabilities(@Nullable Integer capabilitiesId) {
        return CAPABILITIES_DATABASE.getOrDefault(capabilitiesId != null ? capabilitiesId.intValue() : -1,
                new Capabilities());
    }

    /**
     * Return a Capabilities class instance that corresponds to the given 'typeId' parameter. If the 'typeId' parameter
     * is a valid type in the database, and it has a 'capabilitiesOverride' value, then an instance of the respective
     * overridden Capabilities class is returned. Otherwise if the 'capabilitiesId' parameter is for a valid
     * capabilities entry in the database, then that respective Capabilities class instance is returned. Otherwise a
     * blank Capabilities class instance is returned.
     *
     * @param typeId the target shade type Id (to check if it has a 'capabilitiesOverride' value).
     * @param capabilitiesId the target capabilities value (when type Id does not have a 'capabilitiesOverride').
     * @return corresponding Capabilities class instance.
     */
    public Capabilities getCapabilities(int typeId, @Nullable Integer capabilitiesId) {
        int targetCapabilities = TYPE_DATABASE.getOrDefault(typeId, new Type()).getCapabilitiesOverride();
        if (targetCapabilities < 0) {
            targetCapabilities = capabilitiesId != null ? capabilitiesId.intValue() : -1;
        }
        return getCapabilities(targetCapabilities);
    }

And furthermore, I propose the following change (not yet committed, awaiting your feedback)..

4. I think we should DELETE the secondaryRailDetected and tiltAnywhereDetected properties from the UI; these depend on dynamic data from the JSON positions element, so the UI display may actually change depending on the history of commands sent to or received from the shade. In my opinion if a user has a problem that would require us to know these 'properties', then we can (and probably should) get the information via log:set TRACE.
5. Or alternatively if you insist to retain these properties, then to be fully logical we should also add the other dynamic properties tiltOnCloseDetected / tilt180Detected / overlappedSecondaryDetected to be consistent.

[jlaur]

• I think we should DELETE the secondaryRailDetected and tiltAnywhereDetected properties from the UI; these depend on dynamic data from the JSON positions element, so the UI display may actually change depending on the history of commands sent to or received from the shade. In my opinion if a user has a problem that would require us to know these 'properties', then we can (and probably should) get the information via log:set TRACE.
• Or alternatively if you insist to retain these properties, then to be fully logical we should also add the other dynamic properties tiltOnCloseDetected / tilt180Detected / overlappedSecondaryDetected to be consistent.

I'm totally fine with removing them as they don't provide much value as you also conclude, and could even cause unneeded confusion.

[jlaur]

• I think we should DELETE the secondaryRailDetected and tiltAnywhereDetected properties from the UI;

I'm totally fine with removing them as they don't provide much value as you also conclude, and could even cause unneeded confusion.

@andrewfg - do you want to include it in this PR or another one?

[andrewfg]

do you want to include it in this PR or another one?

@jlaur I will remove them now :)

[andrewfg]

@jlaur the properties have now been hidden :)

[jlaur]

One last thing... :-)

[jlaur]

Thanks for these improvements!

[andrewfg]

@jlaur thanks to you too :)