refactor(inovelli): extract shared logic to src/lib/inovelli.ts#11761
Open
rohankapoorcom wants to merge 2 commits intoKoenkk:masterfrom
Open
refactor(inovelli): extract shared logic to src/lib/inovelli.ts#11761rohankapoorcom wants to merge 2 commits intoKoenkk:masterfrom
rohankapoorcom wants to merge 2 commits intoKoenkk:masterfrom
Conversation
Move types, constants, attribute maps, helpers, extend, and converters into a new lib; export cluster names, VZM*_ATTRIBUTES, and m (extend). Device file is now ~130 lines with five definitions only. Add docs for attribute composition and resolveEndpointAndKey; drop unused @ts-expect-error in led_effect converter. Co-authored-by: Cursor <cursoragent@cursor.com>
…-herdsman compatibility Made-with: Cursor
Contributor
Author
|
@InovelliUSA This probably needs fairly extensive testing from your side. AFAIK, there should be no code functionality changes, just a whole bunch of refactoring in place. Of course we have no unit tests to validate functionality... so 😀 |
Contributor
|
@rohankapoorcom I will run this on my network, but let me know if you make any modifications to it so I can update it in my environment. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the second piece in the next phase of my refactor for the Inovelli converters. Cleaning this up moves all of Inovelli's functionality over to a new file in /src/lib/ and leaves behind the device specific configuration in the converter file itself.
This refactor was planned and executed with assistance from Cursor.
Execution plan (click to expand)
Inovelli lib cleanup plan
Current state
Inovelli,InovelliMmWave)inovelliExtend(device/addCluster/light/fan/MMWave/energyReset)COMMON_ATTRIBUTES,COMMON_DIMMER_*,VZM30_ATTRIBUTES…VZM36_ATTRIBUTES)attributesToExposeList,chunkedRead,intToFanMode,speedToInt,createMmWaveCompositeAreaConverter)fzLocal) and toZigbee (tzLocal) convertersexposeLedEffects,exposeBreezeMode,exposeMMWave*, etc.)definitionsexport (5 devices: VZM30-SN, VZM31-SN, VZM32-SN, VZM35-SN, VZM36)Target pattern (from existing libs)
m,tz,fz, and helpers. src/devices/philips.ts imports* as philips from "../lib/philips"and only defines devices (plus device-onlytzLocalwhere needed).ledvanceFz,ledvanceTz, and extend helpers; device file imports and composes.ikeaLight(),ikeaBattery(), etc.; device file imports and uses them.Approach
Create src/lib/inovelli.ts and move all shared logic into it; keep src/devices/inovelli.ts as a thin file that imports from the lib and only declares the five device definitions.
1. Add
src/lib/inovelli.tsMove the following from the device file into the new lib (preserving structure and types):
Zclfrom zigbee-herdsman,Parametertype,fz/tzfrom converters,exposes,m(modernExtend),reporting, types from../lib/types,utils, and forchunkedRead/ cluster reads:Zh(endpoint type).Inovelli,InovelliMmWave,Attribute,BreezeModeValues.clickLookup,buttonLookup,ledEffects,individualLedEffects,mmWaveControlCommands,INOVELLI_CLUSTER_NAME,INOVELLI_MMWAVE_CLUSTER_NAME,INOVELLI(0x122f),FAN_MODES,BREEZE_MODES,LED_NOTIFICATION_TYPES,BUTTON_TAP_SEQUENCES.COMMON_ATTRIBUTESis the base;COMMON_DIMMER_ATTRIBUTES,COMMON_DIMMABLE_LIGHT_ATTRIBUTES, andCOMMON_DIMMER_ON_OFF_ATTRIBUTESextend it; eachVZM*_ATTRIBUTEScomposes from these with device-specific overrides (e.g. VZM31 temporarily excludesfanTimerMode). Add a short comment block in the lib documenting this layering. Export only the device-facing attribute sets:VZM30_ATTRIBUTES,VZM31_ATTRIBUTES,VZM32_ATTRIBUTES,VZM32_MMWAVE_ATTRIBUTES,VZM35_ATTRIBUTES,VZM36_ATTRIBUTES. KeepCOMMON_* internal (not exported) so the device file only usesinovelli.VZM30_ATTRIBUTES, etc.; composition stays an implementation detail and the API stays small.intToFanMode,speedToInt,attributesToExposeList,chunkedRead,createMmWaveCompositeAreaConverter.inovelliExtendobject (addCustomClusterInovelli, addCustomMMWaveClusterInovelli, inovelliDevice, inovelliLight, inovelliFan, inovelliMMWave, inovelliEnergyReset).tzLocalandfzLocal(move as-is; they reference the constants and types above).exposeLedEffects,exposeIndividualLedEffects,exposeBreezeMode,exposeMMWaveControl,exposeLedEffectComplete,exposeEnergyReset,exposeMMWaveAreas,createAreaComposite,exposeDetectionAreas,exposeInterferenceAreas,exposeStayAreas.Exports from the lib (follow philips/ledvance style):
CLUSTER_NAMEandMMWAVE_CLUSTER_NAMEso the device file usesinovelli.CLUSTER_NAMEandinovelli.MMWAVE_CLUSTER_NAME. Internally the lib can keepINOVELLI_CLUSTER_NAME/INOVELLI_MMWAVE_CLUSTER_NAMEand re-export:export const CLUSTER_NAME = INOVELLI_CLUSTER_NAMEandexport const MMWAVE_CLUSTER_NAME = INOVELLI_MMWAVE_CLUSTER_NAME.export { inovelliExtend as m }(device file uses onlyinovelli.m.*).VZM30_ATTRIBUTES,VZM31_ATTRIBUTES,VZM32_ATTRIBUTES,VZM32_MMWAVE_ATTRIBUTES,VZM35_ATTRIBUTES,VZM36_ATTRIBUTES(device file usesinovelli.VZM30_ATTRIBUTES, etc.). Do not exportCOMMON_ATTRIBUTESorCOMMON_DIMMER_* /COMMON_DIMMABLE_LIGHT_* /COMMON_DIMMER_ON_OFF_*; they remain internal building blocks.Use a single
const e = exposes.presetsandconst ea = exposes.access(andexposeswhere needed) inside the lib file. The lib will depend on../converters/fromZigbee,../converters/toZigbee,../lib/exposes,../lib/modernExtend,../lib/reporting,../lib/types,../lib/utils; path adjustments: fromsrc/lib/inovelli.tsuse../converters/...and./...for other libs.2. Shrink
src/devices/inovelli.tsimport * as inovelli from "../lib/inovelli";import * as m from "../lib/modernExtend";DefinitionWithExtendif not re-exported from lib).definitionsarray. Each definition should:inovelli.m.* for extend (e.g.inovelli.m.addCustomCluster(),inovelli.m.device(...),inovelli.m.light(), etc. — see naming convention below).inovelli.CLUSTER_NAMEandinovelli.MMWAVE_CLUSTER_NAMEwhere cluster names are required (no double "Inovelli").inovelli.VZM30_ATTRIBUTES,inovelli.VZM31_ATTRIBUTES, etc. where attribute sets are required.inovelli.fz.* andinovelli.tz.* for any converters referenced in definitions (e.g. VZM36'sfromZigbee/toZigbeethat referencefzLocal.brightness,fzLocal.vzm36_fan_light_state,tzLocal.vzm36_fan_on_off).inovelliExtend,tzLocal,fzLocal, and all expose helpers. No duplicate logic should remain.Result: the device file is on the order of ~130 lines (imports + five definition objects).
3. Naming and export convention
CLUSTER_NAMEandMMWAVE_CLUSTER_NAMEso usage isinovelli.CLUSTER_NAME/inovelli.MMWAVE_CLUSTER_NAME(Inovelli appears only once, in the namespace).addCustomClusterInovelli→addCustomClusteraddCustomMMWaveClusterInovelli→addCustomMMWaveClusterinovelliDevice→deviceinovelliLight→lightinovelliFan→faninovelliMMWave→mmWaveinovelliEnergyReset→energyResetSo usage is
inovelli.m.device(...),inovelli.m.light(),inovelli.m.addCustomCluster(), etc.4. tz/fz export: do we need to export individual converters?
No. Keep tz/fz internal to the lib.
All five devices (VZM30-SN through VZM36) now use only extend at the definition level. VZM36 has been refactored to use the same pattern as the others:
extend: [inovelli.m.light({ splitValuesByEndpoint: true }), inovelli.m.fan({ endpointId: 2, splitValuesByEndpoint: true }), inovelli.m.device(...), inovelli.m.addCustomCluster(), m.identify()]withfromZigbee: []andtoZigbee: []. The extend methods push the right converters internally (e.g.on_off_for_endpoint,brightness, parameterizedfan_state(endpointId)). The device file never references individual converters; the lib's extend (m) is the only public API, and tz/fz stay internal (not exported).5. Attribute objects (handling the big composed objects)
COMMON_ATTRIBUTES→COMMON_DIMMER_ATTRIBUTES,COMMON_DIMMABLE_LIGHT_ATTRIBUTES,COMMON_DIMMER_ON_OFF_ATTRIBUTES→ device-specificVZM*_ATTRIBUTES(with spreads and overrides). No refactor of the composition logic.VZM30_ATTRIBUTES,VZM31_ATTRIBUTES,VZM32_ATTRIBUTES,VZM32_MMWAVE_ATTRIBUTES,VZM35_ATTRIBUTES,VZM36_ATTRIBUTES. The device file never referencesCOMMON_*; those stay internal.6. Cleanup inside converters (optional, within scope of move)
_, if two parts use endpoint N and base key" is duplicated ininovelli_parameters(convertSet + convertGet) andinovelli_parameters_readOnly(convertGet). Add a small helper in the lib, e.g.resolveEndpointAndKey(entity, key, meta): { entityToUse, keyToUse }, and use it in those three places so the logic lives in one place and is easier to maintain.// @ts-expect-error ignoreand// XXX: far too dynamic to properly typeas-is during the move (no behavior or type changes). These are known limits; improving them can be a follow-up if desired.7. Staged rollout (multiple PRs for easier review)
Break the work into four stages. Each stage is a separate, mergeable change that leaves the codebase buildable and tests passing. Later stages depend on earlier ones.
Stage 1: Create lib with types, constants, attributes, and pure helpers
Inovelli,InovelliMmWave,Attribute,BreezeModeValues.INOVELLI_CLUSTER_NAME/INOVELLI_MMWAVE_CLUSTER_NAME),INOVELLI,clickLookup,buttonLookup,ledEffects,individualLedEffects,mmWaveControlCommands,FAN_MODES,BREEZE_MODES,LED_NOTIFICATION_TYPES,BUTTON_TAP_SEQUENCES.COMMON_* internal,VZM*_ATTRIBUTEScomposed as today) plus a short comment documenting the composition. Export onlyVZM30_ATTRIBUTES…VZM36_ATTRIBUTES.intToFanMode,speedToInt,attributesToExposeList,chunkedRead,createMmWaveCompositeAreaConverter(and exportattributesToExposeList,chunkedReadif the device file still needs them in this stage).CLUSTER_NAME,MMWAVE_CLUSTER_NAME,INOVELLI, lookups (or only what the device file needs),VZM30_ATTRIBUTES…VZM36_ATTRIBUTES, and typesAttribute,Inovelli,InovelliMmWave,BreezeModeValuesso the device file can type annotations. Export pure helpers that the device file's extend will call.import * as inovelli from "../lib/inovelli". Replace every use of the moved constants, attribute maps, types, and pure helpers withinovelli.*. Delete the corresponding definitions from the device file (interfaces, constants, attribute maps, pure helpers). LeaveinovelliExtend,tzLocal,fzLocal, and expose helpers in the device file; they now referenceinovelli.CLUSTER_NAME,inovelli.ledEffects, etc.Stage 2: Move extend object and expose helpers to lib
device,light,fan,mmWave,energyResetwith the planned method renames) and all expose helpers (includingexposeMMWaveTargets). Export the extend asm.inovelliExtend.* call withinovelli.m.* (using the new method names). VZM36 already uses only extend (same pattern as others:light({ splitValuesByEndpoint: true }),fan({ endpointId: 2, splitValuesByEndpoint: true }),device(...),addCustomCluster(),m.identify()) with emptyfromZigbee/toZigbee. Remove the localinovelliExtendand all expose helpers.inovelli.m.*. No behavior change.Stage 3: Move converters to lib (and optional DRY helper)
tzLocalandfzLocal(includingon_off_for_endpoint,fan_state(endpointId),report_target_info, etc.); they are used only internally by the extend. Do not exporttz/fz. Optionally addresolveEndpointAndKey(entity, key, meta)and use it in the three places that duplicate the endpoint/key logic for split endpoints.tzLocalandfzLocal(no definitions reference them; all devices use only extend).Stage 4: Final polish
pnpm run build,pnpm run check,pnpm test. Optionally run a quick smoke check that a device (e.g. VZM31-SN) still resolves and exposes as before.Summary of stages
m). Device file: uses inovelli.m.*; removes local extend + expose helpers. Mergeable: Yes.After Stage 3, the device file is minimal (~130 lines). Stage 4 is optional polish and can be folded into Stage 3 if you prefer fewer PRs.
8. Verification (per stage and final)
pnpm run build,pnpm run check, andpnpm test; fix any path/type or test failures before merging.9. Summary
exposeMMWaveTargets). Exports:CLUSTER_NAME,MMWAVE_CLUSTER_NAME, attribute maps,m(extend with methodsdevice,light,fan,addCustomCluster,addCustomMMWaveCluster,mmWave,energyReset). tz/fz stay internal; no device references them.../lib/inovelliand../lib/modernExtend; only the fivedefinitionsentries. All devices (including VZM36) use onlyinovelli.m.* (light, fan, device, addCustomCluster, mmWave, energyReset with appropriate options),inovelli.CLUSTER_NAME,inovelli.MMWAVE_CLUSTER_NAME, andinovelli.VZM*_ATTRIBUTES. No references toinovelli.tzorinovelli.fz.This matches the existing "vendor lib + thin device file" pattern used by philips, ikea, and ledvance and keeps device definitions in one place while making Inovelli logic reusable and testable from
src/lib/inovelli.ts.