AutomaticInboxProcessor registered as an RegistryListener #4192
Conversation
…ng registry to handle REMOVED events correctly Fixes: eclipse-archived#4187 Signed-off-by: Andre Fuechsel <andre.fuechsel@telekom.de>
| @@ -78,6 +79,8 @@ public void receiveTypedEvent(ThingStatusInfoChangedEvent event) { | |||
| } | |||
| } | |||
|
|
|||
| // InboxListemer methods | |||
There was a problem hiding this comment.
Do we need such marker? I don't think we used it on all the other places.
There was a problem hiding this comment.
No, will remove it. :)
| @@ -278,6 +278,7 @@ private void onConnectionResumed() throws IOException, ApiException { | |||
| properties.put(Thing.PROPERTY_SERIAL_NUMBER, config.getMACAddress().replaceAll(":", "").toLowerCase()); | |||
| properties.put(Thing.PROPERTY_FIRMWARE_VERSION, config.getSoftwareVersion()); | |||
| updateProperties(properties); | |||
| updateThing(thing); | |||
There was a problem hiding this comment.
How is this change related to the AutomaticInboxProcessor registered as an RegistryListener?
There was a problem hiding this comment.
If the properties would not be persisted, the automatic inbox listener would be unable to retrieve the representation property during deletion. Please note, that this was on purpose as long as the hue binding was updating the properties in each polling cycle. Now that it is guarded, that problem won't exist anymore and the properties should be persisted.
There was a problem hiding this comment.
Just came across this here. How does it relate to #1682 and esp. #588 (comment)?
There was a problem hiding this comment.
@SJKA I see your point but I wonder how to solve this issue without persisting the properties. As the thing is already gone when the removed callback is called I would not be able to get the property value. Hmm.
There was a problem hiding this comment.
@afuechsel I is always a bad smell to do some specific code changes in one binding for a concept that should be applicable in the whole framework (and thus be the same for all bindings). Note that after #1682 had been implemented, we meanwhile had #2629 in place. This allows to do Thing updates (structure AND configuration) and persists those updates in case it is a managed Thing. Imho, what is missing in that PR is that it also tries to persist properties in the very same way. If this was done, it would be in line with how configuration updates are handled and it would be exactly what you need here (i.e. this line can be removed as the framework already takes care of everything). Do you agree?
There was a problem hiding this comment.
I think, if properties would be persisted in some other way, this line could be removed. But did I understand correctly, that there is still some tiny piece missing (persisting the properties)? Would that mean to revert #1682?
There was a problem hiding this comment.
Well, then this PR needs to be changed too.
There was a problem hiding this comment.
Ah, didn't see it in the first place :)
| @@ -136,6 +136,7 @@ private synchronized void initializeProperties() { | |||
| } | |||
| updateProperty(LIGHT_UNIQUE_ID, fullLight.getUniqueID()); | |||
| isOsramPar16 = OSRAM_PAR16_50_TW_MODEL_ID.equals(modelId); | |||
| updateThing(thing); | |||
There was a problem hiding this comment.
How is this change related to the AutomaticInboxProcessor registered as an RegistryListener?
| @@ -78,6 +79,8 @@ public void receiveTypedEvent(ThingStatusInfoChangedEvent event) { | |||
| } | |||
| } | |||
|
|
|||
| // InboxListemer methods | |||
There was a problem hiding this comment.
already removed as @maggu2810 suggested
Signed-off-by: Andre Fuechsel <andre.fuechsel@telekom.de>
for the thing registry to handle REMOVED events correctly
Fixes: #4187
Signed-off-by: Andre Fuechsel andre.fuechsel@telekom.de