Koenkk / zigbee-herdsman-converters

Collection of device converters to be used with zigbee-herdsman
MIT License
858 stars 2.83k forks source link

Allow `exposes` in `ModernExtend` interface to be a function #7683

Closed burmistrzak closed 1 week ago

burmistrzak commented 1 week ago

Already works in Definition, now it does in ModernExtend as well!

Context here: https://github.com/Koenkk/zigbee-herdsman-converters/pull/7444#issuecomment-2183967705

burmistrzak commented 1 week ago

@Koenkk Huh, that's interesting... Why doesn't this just work? I guess, because some libs are expecting Expose[]? πŸ€”

burmistrzak commented 1 week ago

@Koenkk Hmm, we still have some type mismatches here and there... Maybe there's actually a good reason why modernExtend only uses Expose[], that I'm simply not aware of?

We don't necessarily have to change the interface, IMHO. As long as we somehow figure out how to dynamically update exposes in a ModernExtend based on Zh.Device stuff, I'll finally be able to finish up the conversion of the BMCT-SLZ. 😊

Koenkk commented 1 week ago

I've implemented it in https://github.com/Koenkk/zigbee-herdsman-converters/commit/059dfb88464287ac63da5cff880db97ce1fe1ba4 now, so this can be closed.

burmistrzak commented 1 week ago

@Koenkk Oh nice, thank you! ☺️ I would have made the required changes myself, but I first wanted to make sure there're be no sneaky surprises. Glad you've figured it out.

burmistrzak commented 4 days ago

@Koenkk What do you think, did I get that exposes right? Do I need some additional arguments somewhere? πŸ€”

bmct: (): ModernExtend => {
    const fromZigbee: Fz.Converter[] = [
        // ...
    ];
    const toZigbee: Tz.Converter[] = [
        // ...
    ];
    const exposes: DefinitionExposesFunction[] = [
        (device: Device, options: KeyValue): Expose[] => {
            const expose: Expose[] = [
                e.linkquality(),
            ];
            // Doesn't use 'device' yet, but you get the idea.
            return expose;
        },
    ];
    return {
        fromZigbee,
        toZigbee,
        exposes,
        isModernExtend: true,
    };
},
Koenkk commented 3 days ago

Yes this looks good, does it work?

burmistrzak commented 3 days ago

Yes this looks good, does it work?

@Koenkk We'll know soon enough! Just have to reset a BMCT-SLZ first. Hopefully the Expose[] gets updated correctly. 🀞