Add support of `Startable`, `Stoppable` and `Destroyable` to LwM2mObjectEnabler. #940

moznion · 2020-11-26T15:10:22Z

Done

Moves Destroyable, Startable and Stoppable class to leshan-core from leshan-server-core to use these classes with LwM2mObjectEnabler
Implements Destroyable, Startable and Stoppable on LwM2mObjectTree
- These implemented methods call the associated objectEnablers' method inside
Calls LwM2mObjectTree#{stop(), start(), destroy()} in LeshanClient#{stop(), start(), destroy()}

sbernard31

(Another small details, generally I try to keep commit title/summary to 72 characters max, if you need to add more information just add a blank line then a more complete description without size limitation)

sbernard31 · 2020-11-26T15:17:37Z

leshan-client-core/src/main/java/org/eclipse/leshan/client/resource/LwM2mObjectEnabler.java

@@ -55,6 +58,11 @@
 * <p>
 * In case you really need the flexibility of this interface you should consider to inherit from
 * {@link BaseObjectEnabler}.
+ * <p>
+ * If you need to destroy or stop this instance on LeshanClient stopped/destroyed,
+ * please also implement {@link Destroyable} and/or {@link Stoppable}.


A little detail but I think Implementing Stoppable without Startable doesn't make too much sense.
Because either you destroy because you don't want to be able to restart or you stop which let you able to restart.

thanks, reworded at bd7f86d

sbernard31 · 2020-11-26T15:19:40Z

leshan-client-cf/src/main/java/org/eclipse/leshan/client/californium/LeshanClient.java

+            if (objectEnabler instanceof Startable) {
+                ((Startable) objectEnabler).start();
+            }
+        }


An idea is to add a start/stop/destroy method to ~~LwM2mObjectEnabler~~ LwM2mObjectTree and move the loop in this class ?
(This is maybe better in term of separation of concern)

Thank you. That sounds good, I'll do that.

@sbernard31 Do you mean LwM2mObjectTree instead of LwM2mObjectEnabler ?

Uups, I wanted to say LwM2mObjectTree 😅

haha, thanks for your advice! I did this at 603cb2a

sbernard31 · 2020-11-26T15:29:28Z

leshan-client-cf/src/main/java/org/eclipse/leshan/client/californium/LeshanClient.java

+            if (objectEnabler instanceof Destroyable) {
+                ((Destroyable) objectEnabler).destroy();
+            }
+        }


For destroy there is special case where we implement Stoppable but not Destroyable. See LeshanServer

Gotcha, I'm going to fix this.

fixed at bd7f86d

Move the interfaces `Destroyable`, `Startable` and `Stoppable` to `leshan-core` package from `leshan-server-core`. This commit aims to use these interfaces with `LwM2mObjectEnabler` to control the state of that. Ref: eclipse-leshan#933 (comment) Signed-off-by: moznion <[email protected]>

Now it calls `destroy()`, `stop()` and `start()` for each LwM2mObjectEnabler instance according to the LeshanClient status. For example, If it calls `LeshanClient#destroy()`, that method calls for each LwM2mObjectEnabler's `#destroy()`. And other methods are also similar. Signed-off-by: moznion <[email protected]>

According to this feedback: eclipse-leshan#940 (comment) Signed-off-by: moznion <[email protected]>

If an instance of `LwM2mObjectEnabler` doesn't implement the `Destroyable` but that implements `Stoppable` instead, it calls `#stop()` on `LeshanClient#destroy()` instead of `destroy()`. Signed-off-by: moznion <[email protected]>

moznion · 2020-11-27T11:41:58Z

(Another small details, generally I try to keep commit title/summary to 72 characters max, if you need to add more information just add a blank line then a more complete description without size limitation)

Thank you for your suggestion. I rebased this branch and reworded the long commit summary.

These implemented methods are used in associated LeshanClient methods. Signed-off-by: moznion <[email protected]>

sbernard31

👍

sbernard31 · 2020-11-27T14:45:35Z

Integrated in master (commit fa0f58c and a275f2b)

I did some little adjustment :

squashed 4 last commits in one.
added the Better handle unexpected error in DefaultRegistrationEngine. #933 issue reference in commit title.
fixing some formatting issue in javadoc of LwM2mObjectEnabler.
removing unused import (in LeshanClient)
(About formatting and unused import, eclipse can help you for this)

Thx @moznion 🙏 !

moznion · 2020-11-27T14:47:05Z

Many thanks for your supporting, @sbernard31 !

sbernard31 reviewed Nov 26, 2020

View reviewed changes

sbernard31 changed the title ~~Move some classes into core and call start(), stop() and destroy() for each LwM2mObjectEnabler instance according to the status of LeshanClient~~ Add support of Startable, Stoppable and Destroyable to LwM2mObjectEnabler. Nov 26, 2020

moznion added 3 commits November 27, 2020 13:39

Reword javadoc for LwM2mObjectEnabler interface

bd7f86d

According to this feedback: eclipse-leshan#940 (comment) Signed-off-by: moznion <[email protected]>

moznion force-pushed the 933_better_handle_unexpected_error_move_package branch from 05b7dd6 to b1cc299 Compare November 27, 2020 11:39

moznion force-pushed the 933_better_handle_unexpected_error_move_package branch from b1cc299 to c334876 Compare November 27, 2020 11:40

Implement Startable, Stoppable and Destroyable on LwM2mObjectTree

603cb2a

These implemented methods are used in associated LeshanClient methods. Signed-off-by: moznion <[email protected]>

moznion requested a review from sbernard31 November 27, 2020 14:18

sbernard31 approved these changes Nov 27, 2020

View reviewed changes

sbernard31 closed this Nov 27, 2020

moznion deleted the 933_better_handle_unexpected_error_move_package branch November 27, 2020 14:46

moznion mentioned this pull request Dec 8, 2020

Backport: error handling for unexpected error in DefaultRegistrationEngine #944

Closed

sbernard31 mentioned this pull request Dec 10, 2020

Better handle unexpected error in DefaultRegistrationEngine. #933

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support of `Startable`, `Stoppable` and `Destroyable` to LwM2mObjectEnabler. #940

Add support of `Startable`, `Stoppable` and `Destroyable` to LwM2mObjectEnabler. #940

moznion commented Nov 26, 2020 •

edited

Loading

sbernard31 left a comment

sbernard31 Nov 26, 2020

moznion Nov 27, 2020

sbernard31 Nov 26, 2020 •

edited

Loading

moznion Nov 27, 2020

moznion Nov 27, 2020

sbernard31 Nov 27, 2020

moznion Nov 27, 2020

sbernard31 Nov 26, 2020 •

edited

Loading

moznion Nov 27, 2020

moznion Nov 27, 2020

moznion commented Nov 27, 2020

sbernard31 left a comment

sbernard31 commented Nov 27, 2020

moznion commented Nov 27, 2020

Add support of Startable, Stoppable and Destroyable to LwM2mObjectEnabler. #940

Add support of Startable, Stoppable and Destroyable to LwM2mObjectEnabler. #940

Conversation

moznion commented Nov 26, 2020 • edited Loading

Done

sbernard31 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbernard31 Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbernard31 Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moznion commented Nov 27, 2020

sbernard31 left a comment

Choose a reason for hiding this comment

sbernard31 commented Nov 27, 2020

moznion commented Nov 27, 2020

Add support of `Startable`, `Stoppable` and `Destroyable` to LwM2mObjectEnabler. #940

Add support of `Startable`, `Stoppable` and `Destroyable` to LwM2mObjectEnabler. #940

moznion commented Nov 26, 2020 •

edited

Loading

sbernard31 Nov 26, 2020 •

edited

Loading

sbernard31 Nov 26, 2020 •

edited

Loading