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

[miele] Clean up properties and improve reliability and performance #11423

Merged
merged 26 commits into from
Nov 1, 2021

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Oct 23, 2021

Fixes #11422

Signed-off-by: Jacob Laursen jacob-github@vindvejr.dk

Reliability

#11244 introduced a dependency to the protocol property for correctly handling both ZigBee and Wi-Fi appliances. This had two drawbacks:

  • The protocol property is a technical protocol prefix ("hdm:ZigBee:" or "hdm:LAN:"). [miele] Fix multicast and multi-protocol support (ZigBee/Wi-Fi) #11244 did not change this, but it would make more sense to expose a user-oriented protocol name like "ZigBee" or "Wi-Fi". This is not possible with current technical dependency.
  • In rare scenarios properties are not correctly set. This breaks channel updates as correct API calls cannot be made without protocol prefix. I'm not sure exactly when this happens, but it could be timing related if properties are not initialized before receiving state updates.

Similarly a dependency to the serialNumber property was introduced for correct handling multicast updates.

To fix both problems, two internal appliance caches are now used for fast lookup of UID from appliance id (i.e. "0123456789#0" -> "hdm:ZigBee:0123456789#0") as well as from remote UID/serial number. After removing this property dependency, the protocol property has been renamed to protocolAdapter which now contains only the actual protocol adapter name, i.e. ZigBee or LAN. Additionally, a new property, connectionType, has been introduced. For Wi-Fi devices protocolAdapter is LAN and connectionType is WiFi.

Performance

Appliance cache has been changed from List to HashMap for faster lookups, and is also being used more actively now for looking up UID's.

Property cleanup

Properties removed:

  • brandId always containing value MI.
  • companyId always containing value MI.

Properties renamed:

  • dc renamed to deviceClass. Now also set for things configured in files, where previously only auto-discovered things had this property set.
  • protocol renamed to protocolAdapter as this is more correct.

Properties added:

  • model containing the appliance model.
  • connectionType (not available for ZigBee appliances).

Before:
image

After:
image

Refactoring

Refer to "gateway" instead of "ZigBee network" in configuration, since both ZigBee and LAN/Wi-Fi appliances are supported by the gateway.

Added constants for remaining handlers with hardcoded device classes.

Changed obscure string operations for skipping this result of the getDeviceClassObjects response:
"DeviceClass": "com.prosyst.mbs.services.zigbee.hdm.deviceclasses.ReportingControl"

while processing only this result:
"DeviceClass": "com.miele.xgw3000.gateway.hdm.deviceclasses.MieleWashingMachine"
(or other device class name for appliance)

Fixed all warnings:

  • Potential null pointer access
  • Redundant null check
  • Import never used

Fixed some Static Code Analysis issues:

  • The package org.apache.commons.lang3.StringUtils should not be used.
  • 'static' modifier out of order with the JLS suggestions.
  • First javadoc author should have "Initial contribution" contribution description.

Translation

Added default i18n properties file and partial Danish translation as baseline. Next steps will be adding custom strings for states, programs and phases so these can be provided in desired language. This is not planned within the context of this PR.

… of relying on property.

Fixes openhab#11422

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur requested a review from kgoderis as a code owner October 23, 2021 08:24
@jlaur jlaur marked this pull request as draft October 23, 2021 08:24
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
…rom auto-discovered things).

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur
Copy link
Contributor Author

jlaur commented Oct 23, 2021

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur changed the title [miele] Performance and reliability improvements [miele] Clean up properties and improve reliability and performance Oct 23, 2021
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur marked this pull request as ready for review October 23, 2021 21:34
@jlaur jlaur marked this pull request as draft October 24, 2021 13:06
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur marked this pull request as ready for review October 24, 2021 15:13
@jlaur jlaur marked this pull request as draft October 25, 2021 11:34
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur jlaur marked this pull request as ready for review October 25, 2021 15:27
…ay, although this is a quite rare scenario.

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
@jlaur
Copy link
Contributor Author

jlaur commented Oct 30, 2021

@kgoderis, @kaikreuzer - would it be possible for you to review this PR? It would be nice if it could be included in next milestone as it fixes an issue with dependency to protocol and seriaNumber properties which really would be nice to solve sooner than later.

Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

Many thanks @jlaur for this nice refactoring and for cleaning up so many places in the code along the way!
Looks all good to me, so let's merge!

@kaikreuzer kaikreuzer merged commit 745bf76 into openhab:main Nov 1, 2021
@kaikreuzer kaikreuzer added the bug An unexpected problem or unintended behavior of an add-on label Nov 1, 2021
@kaikreuzer kaikreuzer added this to the 3.2 milestone Nov 1, 2021
dschoepel pushed a commit to dschoepel/openhab-addons that referenced this pull request Nov 9, 2021
…penhab#11423)

* Use appliance cache for getting full UID with protocol prefix instead of relying on property.
* Set bare protocol name as property.
* Fix potential null pointer access warnings.
* Remove unused import.
* Renamed property protocol to protocolAdapter for correctness.
* Add connectionType property.
* Add appliance model property.
* Remove useless properties brandId and companyId always having value MI.
* Rename property dc to deviceClass and set it consistently (not only from auto-discovered things).
* Added constants for remaining handlers with hardcoded device classes.
* Fix SCA: AuthorContributionDescriptionCheck
* Fix SCA: ModifierOrderCheck
* Rename ExtendedDeviceStateUtil to be a bit more generic.
* Extract device class string parsing to utility method.
* Fix SCA: ForbiddenPackageUsageCheck
* Fix redundant null check.
* Fix potential null pointer access warnings.
* Fix unsafe null type conversion (type annotations)
* Share same configuration (UID) for all appliance types.
* Refer to gateway instead of ZigBee network in configuration.
* Remove dependency to seriaNumber property for multicast channel updates.
* Simplified filtering of irrelevant device class.
* Remove devices from remoteUid cache also when disappearing from gateway, although this is a quite rare scenario.
* Add default i18n properties file.
* Add partial Danish translation.

Fixes openhab#11422

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Dave J Schoepel <dave@theschoepels.com>
jpg0 pushed a commit to jpg0/openhab-addons that referenced this pull request Nov 10, 2021
…penhab#11423)

* Use appliance cache for getting full UID with protocol prefix instead of relying on property.
* Set bare protocol name as property.
* Fix potential null pointer access warnings.
* Remove unused import.
* Renamed property protocol to protocolAdapter for correctness.
* Add connectionType property.
* Add appliance model property.
* Remove useless properties brandId and companyId always having value MI.
* Rename property dc to deviceClass and set it consistently (not only from auto-discovered things).
* Added constants for remaining handlers with hardcoded device classes.
* Fix SCA: AuthorContributionDescriptionCheck
* Fix SCA: ModifierOrderCheck
* Rename ExtendedDeviceStateUtil to be a bit more generic.
* Extract device class string parsing to utility method.
* Fix SCA: ForbiddenPackageUsageCheck
* Fix redundant null check.
* Fix potential null pointer access warnings.
* Fix unsafe null type conversion (type annotations)
* Share same configuration (UID) for all appliance types.
* Refer to gateway instead of ZigBee network in configuration.
* Remove dependency to seriaNumber property for multicast channel updates.
* Simplified filtering of irrelevant device class.
* Remove devices from remoteUid cache also when disappearing from gateway, although this is a quite rare scenario.
* Add default i18n properties file.
* Add partial Danish translation.

Fixes openhab#11422

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
NickWaterton pushed a commit to NickWaterton/openhab-addons that referenced this pull request Dec 30, 2021
…penhab#11423)

* Use appliance cache for getting full UID with protocol prefix instead of relying on property.
* Set bare protocol name as property.
* Fix potential null pointer access warnings.
* Remove unused import.
* Renamed property protocol to protocolAdapter for correctness.
* Add connectionType property.
* Add appliance model property.
* Remove useless properties brandId and companyId always having value MI.
* Rename property dc to deviceClass and set it consistently (not only from auto-discovered things).
* Added constants for remaining handlers with hardcoded device classes.
* Fix SCA: AuthorContributionDescriptionCheck
* Fix SCA: ModifierOrderCheck
* Rename ExtendedDeviceStateUtil to be a bit more generic.
* Extract device class string parsing to utility method.
* Fix SCA: ForbiddenPackageUsageCheck
* Fix redundant null check.
* Fix potential null pointer access warnings.
* Fix unsafe null type conversion (type annotations)
* Share same configuration (UID) for all appliance types.
* Refer to gateway instead of ZigBee network in configuration.
* Remove dependency to seriaNumber property for multicast channel updates.
* Simplified filtering of irrelevant device class.
* Remove devices from remoteUid cache also when disappearing from gateway, although this is a quite rare scenario.
* Add default i18n properties file.
* Add partial Danish translation.

Fixes openhab#11422

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Signed-off-by: Nick Waterton <n.waterton@outlook.com>
nemerdaud pushed a commit to nemerdaud/openhab-addons that referenced this pull request Jan 28, 2022
…penhab#11423)

* Use appliance cache for getting full UID with protocol prefix instead of relying on property.
* Set bare protocol name as property.
* Fix potential null pointer access warnings.
* Remove unused import.
* Renamed property protocol to protocolAdapter for correctness.
* Add connectionType property.
* Add appliance model property.
* Remove useless properties brandId and companyId always having value MI.
* Rename property dc to deviceClass and set it consistently (not only from auto-discovered things).
* Added constants for remaining handlers with hardcoded device classes.
* Fix SCA: AuthorContributionDescriptionCheck
* Fix SCA: ModifierOrderCheck
* Rename ExtendedDeviceStateUtil to be a bit more generic.
* Extract device class string parsing to utility method.
* Fix SCA: ForbiddenPackageUsageCheck
* Fix redundant null check.
* Fix potential null pointer access warnings.
* Fix unsafe null type conversion (type annotations)
* Share same configuration (UID) for all appliance types.
* Refer to gateway instead of ZigBee network in configuration.
* Remove dependency to seriaNumber property for multicast channel updates.
* Simplified filtering of irrelevant device class.
* Remove devices from remoteUid cache also when disappearing from gateway, although this is a quite rare scenario.
* Add default i18n properties file.
* Add partial Danish translation.

Fixes openhab#11422

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
volkmarnissen pushed a commit to volkmarnissen/openhab-addons that referenced this pull request Mar 3, 2022
…penhab#11423)

* Use appliance cache for getting full UID with protocol prefix instead of relying on property.
* Set bare protocol name as property.
* Fix potential null pointer access warnings.
* Remove unused import.
* Renamed property protocol to protocolAdapter for correctness.
* Add connectionType property.
* Add appliance model property.
* Remove useless properties brandId and companyId always having value MI.
* Rename property dc to deviceClass and set it consistently (not only from auto-discovered things).
* Added constants for remaining handlers with hardcoded device classes.
* Fix SCA: AuthorContributionDescriptionCheck
* Fix SCA: ModifierOrderCheck
* Rename ExtendedDeviceStateUtil to be a bit more generic.
* Extract device class string parsing to utility method.
* Fix SCA: ForbiddenPackageUsageCheck
* Fix redundant null check.
* Fix potential null pointer access warnings.
* Fix unsafe null type conversion (type annotations)
* Share same configuration (UID) for all appliance types.
* Refer to gateway instead of ZigBee network in configuration.
* Remove dependency to seriaNumber property for multicast channel updates.
* Simplified filtering of irrelevant device class.
* Remove devices from remoteUid cache also when disappearing from gateway, although this is a quite rare scenario.
* Add default i18n properties file.
* Add partial Danish translation.

Fixes openhab#11422

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
…penhab#11423)

* Use appliance cache for getting full UID with protocol prefix instead of relying on property.
* Set bare protocol name as property.
* Fix potential null pointer access warnings.
* Remove unused import.
* Renamed property protocol to protocolAdapter for correctness.
* Add connectionType property.
* Add appliance model property.
* Remove useless properties brandId and companyId always having value MI.
* Rename property dc to deviceClass and set it consistently (not only from auto-discovered things).
* Added constants for remaining handlers with hardcoded device classes.
* Fix SCA: AuthorContributionDescriptionCheck
* Fix SCA: ModifierOrderCheck
* Rename ExtendedDeviceStateUtil to be a bit more generic.
* Extract device class string parsing to utility method.
* Fix SCA: ForbiddenPackageUsageCheck
* Fix redundant null check.
* Fix potential null pointer access warnings.
* Fix unsafe null type conversion (type annotations)
* Share same configuration (UID) for all appliance types.
* Refer to gateway instead of ZigBee network in configuration.
* Remove dependency to seriaNumber property for multicast channel updates.
* Simplified filtering of irrelevant device class.
* Remove devices from remoteUid cache also when disappearing from gateway, although this is a quite rare scenario.
* Add default i18n properties file.
* Add partial Danish translation.

Fixes openhab#11422

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
andan67 pushed a commit to andan67/openhab-addons that referenced this pull request Nov 6, 2022
…penhab#11423)

* Use appliance cache for getting full UID with protocol prefix instead of relying on property.
* Set bare protocol name as property.
* Fix potential null pointer access warnings.
* Remove unused import.
* Renamed property protocol to protocolAdapter for correctness.
* Add connectionType property.
* Add appliance model property.
* Remove useless properties brandId and companyId always having value MI.
* Rename property dc to deviceClass and set it consistently (not only from auto-discovered things).
* Added constants for remaining handlers with hardcoded device classes.
* Fix SCA: AuthorContributionDescriptionCheck
* Fix SCA: ModifierOrderCheck
* Rename ExtendedDeviceStateUtil to be a bit more generic.
* Extract device class string parsing to utility method.
* Fix SCA: ForbiddenPackageUsageCheck
* Fix redundant null check.
* Fix potential null pointer access warnings.
* Fix unsafe null type conversion (type annotations)
* Share same configuration (UID) for all appliance types.
* Refer to gateway instead of ZigBee network in configuration.
* Remove dependency to seriaNumber property for multicast channel updates.
* Simplified filtering of irrelevant device class.
* Remove devices from remoteUid cache also when disappearing from gateway, although this is a quite rare scenario.
* Add default i18n properties file.
* Add partial Danish translation.

Fixes openhab#11422

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[miele] Improve performance and reliability
2 participants