Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: convert IKEA to modern extend #7220

Merged
merged 52 commits into from
Apr 2, 2024
Merged

Conversation

mrskycriper
Copy link
Contributor

@mrskycriper mrskycriper commented Mar 14, 2024

TODO

  • revise device names for consistency
  • fix incorrectly assigned models
  • sort definitions into regions
  • use modern extends
  • turn converters into modern extends
  • turn configurations into modern extends
  • move IKEA functions and converters to lib/ikea.ts
  • create zigbee2mqtt.io PR with renamed images for changed model names

Additional

Modern extend improvements

Partial solution for Koenkk/zigbee2mqtt#21951

Model changes

  • LED1923R5/LED1925G6 to LED1923R5 (LED1925G6 already exists)
  • ICPSHC24-10EU-IL-1 to ICPSHC24-10EU-IL-1/ICPSHC24-10EU-IL-2 (same Zigbee model)
  • ICPSHC24-30EU-IL-1 to ICPSHC24-30EU-IL-1/ICPSHC24-30EU-IL-2 (same Zigbee model)
  • LED1934G3_E26 and LED1934G3_E27 to LED1934G3 (same bulbs, different sockets)
  • LED1937T5_E26 and LED1937T5_E27 to LED1937T5 (same bulbs, different sockets)
  • LED1903C5/LED1835C6 to LED1835C6 (LED1903C5 is a dumb dimmable LEDARE bulb)

Images for renamed models: Koenkk/zigbee2mqtt.io#2662

@mrskycriper mrskycriper marked this pull request as draft March 14, 2024 23:46
return {configure, isModernExtend: true};
}

export function ikeaAirPurifier(): ModernExtend {
Copy link
Sponsor Contributor

@sjorge sjorge Mar 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably more readable to split these into multiple modernExtends e.g.

  • ikeaAirPurifierPm25
  • ikeaAirPurifierFilterRunTime
  • ikeaAirPurifierControlPanelLight
  • ikeaAirPurifierChildLock
  • ikeaAirPurifierFanSpeed
  • ikeaAirPurifierFanMode

Each just having the expose, fromZigbee, toZigbee and configure that is relevant for the attribute.

I think I actually have a branch with that somewhere but I never finished it because E_NOTIME

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary though? It’s not like this is going to be reused in parts anyway. Everything here comes from the same ikea purifier specific cluster

Copy link
Sponsor Contributor

@sjorge sjorge Mar 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally it won't make much difference, just readability.

I'll probably eventually do it myself, the current blob of code is not very readable. The person who originally wrote it did not do a very good job at keeping it readable. (I wrote it so It's OK to say it without offending anyone)

@mrskycriper mrskycriper mentioned this pull request Mar 19, 2024
19 tasks
@mrskycriper mrskycriper marked this pull request as ready for review March 30, 2024 17:32
@Koenkk
Copy link
Owner

Koenkk commented Mar 31, 2024

@mrskycriper will merge it after the 1 April release.

@Koenkk Koenkk merged commit c5b17c4 into Koenkk:master Apr 2, 2024
2 checks passed
@Koenkk
Copy link
Owner

Koenkk commented Apr 2, 2024

Thanks!

@mrskycriper mrskycriper deleted the modern-ikea branch April 3, 2024 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants