fix(knx_windowcoverings): correct DPT 5.001 position mapping, with invert_position migration for existing devices

[LaurensVanAcker]

Summary

KNX DPT 5.001 for shutter position has inverse semantics from Homey's windowcoverings_set. The knx_windowcoverings driver did not account for this, leaving the position slider permanently inverted for any KNX-standard installation. This PR fixes the read and write paths so the default configuration matches the KNX standard, and adds a per-device invert_position setting plus a one-time migration so that existing devices and their flows keep behaving exactly as before (addressing the review feedback from @ttherbrink).

Fixes #78.

Why

	KNX (DPT 5.001 for shutter position)	Homey (windowcoverings_set)
Fully open (up)	0 / 0%	1.0 / 100%
Fully closed (down)	255 / 100%	0.0 / 0%

The KNX convention is in KNX Standard v2.1 §3.7.2 (DPT 5.001 Scaling) and in the application descriptions of every major shutter actuator manufacturer (MDT JAL series, ABB JRA/S, Gira, Jung, Theben). The Homey convention is documented at https://apps.developer.homey.app/the-basics/devices/best-practices/window-coverings (1.0 = open, 0.0 = closed).

The existing invert_updown advanced setting on knx_windowcoverings correctly handles DPT 1.008 (it inverts both command direction and ga_status). It does not apply to DPT 5.001 reads/writes on ga_height / ga_height_status. As a result, a shutter that is physically fully closed reports 100% in Homey, the position bar sits at the "open" side of the slider, and dragging the slider up physically closes the shutter.

What changes

A new per-device checkbox setting invert_position next to the existing invert_updown, one knob per DPT:

• invert_position = false (default for newly paired devices): the app follows the KNX standard — DPT 5.001 values are inverted on both the read path (ga_height_status → windowcoverings_set) and the write path (windowcoverings_set → ga_height) so they map correctly onto Homey's semantics.
• invert_position = true (set automatically for devices that existed before this change): the legacy mapping — position values pass through unchanged, exactly as the app behaved before. Existing "Set position" flow cards keep doing what they did.

invert_updown (DPT 1.008 up/down command + status) is unchanged and stays orthogonal.

Migration (no breaking change)

Per the review discussion:

• The pair view sets a windowcoverings_set_migrated store flag on newly paired windowcovering devices (as suggested by @ttherbrink), so they start with the KNX-standard mapping.
• On the first onInit after the upgrade, devices without that flag (i.e. everything paired before this version) that have a position groupaddress configured get invert_position = true, and the flag is then set. Their sliders, percentages, and flows behave identically to today — nothing flips on upgrade. Pre-existing devices without ga_height have no position behaviour to preserve, so they simply adopt the standard mapping if a position address is added later.
• Users on KNX-standard actuators can then opt in to the corrected behaviour at any time by unchecking Invert position in the device's advanced settings — no need to remove and re-pair the device. On toggle, the displayed position is flipped in place, so the slider keeps showing the correct physical position immediately.
• Users who had worked around the bug ETS-side keep invert_position = true (the migration default) and are also unaffected.

Drive-by one-liner: the Homey.createDevice error alert in the shared pair view referenced an undefined err (swallowing any pairing error); it now alerts the actual error.

The setting's hint text explains both states, in all 13 app languages.

Testing

The corrected KNX-standard mapping (the invert_position = false path) was tested earlier on a Homey Pro 2023 with KNX-standard rollershutter actuators: position 0% on a physically closed shutter, slider on the "closed" side, up-gesture opens — matching Homey's documented semantics. The legacy path is the app's current release behaviour, unchanged.

For this revision: homey app validate -l publish passes and app.json is regenerated from compose. I'll run the migration path (upgrade of an already-paired device + toggling the setting) on my Homey Pro and report back here.

[ttherbrink]

I understand the intention to fix this bug.
However the breaking change will mean that everyone currently using the blinds in flow will have their blinds working in reverse. This will cause backlash as apps are updated automatically and change logs are not often read.
I suggest we come up with some kind of migration. I will also discuss with a colleague about the possibilities

[LaurensVanAcker]

I understand the intention to fix this bug. However the breaking change will mean that everyone currently using the blinds in flow will have their blinds working in reverse. This will cause backlash as apps are updated automatically and change logs are not often read. I suggest we come up with some kind of migration. I will also discuss with a colleague about the possibilities

@ttherbrink, no the up and down commands stay the same. Those are already correct, it's only the visual presence that's corrected. Unless users are sending percentages instead of UP/DOWN.

[ttherbrink]

That is exactly what i am talking about. This flowcard's behaviour will be flipped, this will break some users flows. E.g. curtains will close in the morning instead of opening.

I have discussed with a colleague, An option like this would probaly work:
(the following code is untested)

   if (groupaddress === this.getStatusAddress('ga_height')) {
      var value = DatapointTypeParser.dim(data);
      if (await this.getStoreValue('windowcoverings_set_migrated') === true && this.settings.invert_updown === false) {
        value = 1 - value;
      }
      // KNX DPT 5.001 for shutter position: 0% = fully open (up), 100% = fully closed (down).
      // Homey windowcoverings_set: 1.0 = open, 0.0 = closed. Invert to match.
      await this.setCapabilityValue('windowcoverings_set', value)
        .catch((knxerror) => {
          this.error('Set windowcoverings_set error', knxerror);
        });
    }

  onCapabilityWindowCoveringSet(value) {
   if (!this.knxInterface || !this.settings.ga_height) {
     return null;
   }

   if (await this.getStoreValue('windowcoverings_set_migrated') === true && this.settings.invert_updown === false) {
     value = value * 255
   } else {
     value = (1 - value) * 255
   }
   // KNX DPT 5.001 for shutter position: 0% = fully open (up), 100% = fully closed (down).
   // Homey windowcoverings_set: 1.0 = open, 0.0 = closed. Invert to match.
   return this.knxInterface.writeKNXGroupAddress(this.settings.ga_height, (1 - value) * 255, 'DPT5')
     .catch((knxerror) => {
       throw new Error(this.homey.__('errors.windowcovering_failed'), knxerror);
     });
 }

Index.html

        case 'windowcovering':
          device.name = 'KNX Windowcovering';
          device.settings = {
            'ga_up_down': $('#windowcover_updown_address').val(),
            'ga_stop': $('#windowcover_stop_address').val(),
            'ga_status': $('#windowcover_status_address').val(),
            'ga_height': $('#windowcover_height_address').val(),
            'ga_height_status': $('#windowcover_height_status_address').val(),
            'macAddress': interfaceMAC,
          };
          device.store = {
            'windowcoverings_set_migrated': true,
          }
          break;

This should set the flag only on newly paired devices. It also makes sure that the invert setting also works on the slider.

[LaurensVanAcker]

Thanks for the review and the migration concern @ttherbrink — agreed, silently flipping behaviour on existing flows isn't acceptable.

I'd like to propose a slightly different angle: instead of a hidden store flag set only at pair time, expose this as a per-device setting invert_position next to the existing invert_updown.

The user-visible mental model would be: "should the app compensate for the legacy position mapping to preserve old flows?"

• invert_position = false → no compensation. App follows KNX DPT 5.001 (0 = open, 255 = closed) and maps to Homey conventions correctly. This is what new pairings get.
• invert_position = true → legacy compensation. App preserves the historical mapping existing flows depend on. Applied via one-time migration on upgrade for already-paired devices.

A few reasons I think this works better than overloading invert_updown or using a hidden store flag:

• "Invert" naming reflects the unnatural case. KNX-standard installs run with false, which reads as the natural default. Following the spec is the baseline; deviating from it is the explicit opt-in.
• Existing users can migrate at their own pace by toggling the setting in the device UI. With a pair-time-only store flag they'd need to remove and re-add every blind to ever get the correct behaviour.
• invert_updown (DPT 1.008) and invert_position (DPT 5.001) stay orthogonal. A user with inverted up/down but standard position (or vice versa) can express that — one knob per DPT.
• Existing flows stay intact on upgrade thanks to the migration setting invert_position = true. No breakage.

Happy to implement this if you agree on the direction — both the read/write changes and the one-time migration in onInit. Or if you'd rather keep #87 minimal and split it into a follow-up PR, that works too.

[LaurensVanAcker]

@ttherbrink implemented as discussed: the fix is now gated behind a per-device invert_position setting (default = KNX standard for new pairings). Your pair-view store flag (windowcoverings_set_migrated) marks new devices; existing devices with a configured height address are migrated once in onInit to invert_position = true, so current users' flows are untouched on upgrade — they can opt in to the corrected mapping from the device's advanced settings at any time, without re-pairing. PR description updated with the details. Ready for another look.

[LaurensVanAcker]

@ttherbrink friendly ping on this one. I see #86, #89 and #90 have landed in release/1.6.0 in the meantime — could this PR still make that release cycle?

The revision from 24 July implements the migration exactly as we discussed: your windowcoverings_set_migrated store flag is set at pair time, and existing devices with a configured height address are migrated once in onInit to invert_position = true, so no current flow changes behaviour on upgrade.

Two small things I can fix if needed:

• This PR still targets master while the merged ones went to release/1.6.0. Shall I rebase and retarget it to release/1.6.0?
• Anything outstanding from your side on the approach, or is it just waiting for a slot in the release?

Happy to do the rebase straight away if that unblocks it.