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

Have DefaultRegistrationEngine try to update reg after start #1579

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ozame
Copy link

@Ozame Ozame commented Feb 2, 2024

As discussed in #1574 , some changes are needed in the Default implementation of RegistrationEngine to support the stopping and starting of Leshan client without re-registration happening every time.

Basically the list of registered servers is checked when start-method is called, and registration update is tried.

Note Will still need to at least update the tests if this is deemed merge-worthy.

- Before this, re-registration would happen on every engine start
@sbernard31
Copy link
Contributor

(I will look at this next week)

@sbernard31
Copy link
Contributor

I looked at this and regarding my comments at #1574 (comment),

I have no clear idea if this is something which should be integrated OR not.
I'm not sure there is obvious use case but maybe this could be a step in adding support to Queue/Offline mode ? 🤔

I still don't know if this should be added.

Pro :

  • the code modification is pretty simple (but we need to add a configuration set way + new tests)n
  • it supports your use case,
  • maybe (really not sure) it could be a first step for implementing Queue mode 🤔

Con :

  • not sure this will be used too much without a real Queue mode feature ? 🤔

The another point, I'm not sure if it can work with all transport layer java-coap implementation ? (because start stop destroy is mainly a californium concept, some libraries has just start/destroy concept)

I adapt some tests to check if it could works with java-coap and it seems OK. (See d0f5e2f). So this point is not really an issue.

Note that if we wanted to support this feature we should rather add new tests instead of modifying existing one.

@sbernard31
Copy link
Contributor

An alternative to this PR : #1574 (comment)

if (dmServer == null) {
// If it failed try client initiated bootstrap
if (!scheduleClientInitiatedBootstrap(NOW))
throw new IllegalStateException("Unable to start client : No valid server available!");
} else {
registerFuture = schedExecutor.submit(new RegistrationTask(dmServer));
// If there exists a registered server already, we try to send a registration update

Choose a reason for hiding this comment

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

I'd actually be sceptical here. If a client is stopped, it should not keep registrations. So this situation should not be allowed.

If somebody truly needs a LWM2M client that supports this, they can either implement their own registration engine or subclass it.

I am not in favor of doing this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thx for taking time to look at this and also for sharing your opinion. 🙏

For now, no plan to integrate this BUT if for some reason we decide to ingrate it this will not be the default behavior.

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