search

eclipse-leshan / leshan

Java Library for LWM2M
https://www.eclipse.org/leshan/
BSD 3-Clause "New" or "Revised" License
647 stars 407 forks source link

Organization for refactoring about transport layer #1222

Open sbernard31 opened 2 years ago

sbernard31 commented 2 years ago

I create this issue to organize the team work.

I plan to begin massive refactoring about "adding a way to add more transport Layer" (#1025).

This will be a lot of changes (with lot of API break) and this will probably take much time. I feel this will be a good idea to not work on complex feature at the same time to avoid to manage error prone conflicts.

So my idea would be to deliver a 2.0.0-M7 with all ongoing work :

Then go for a kind of feature freeze during the transport layer work. I think I will do most of the work in separated branches and integrate this in master once all will be in an acceptable state.

At the same time we can continue to add some bug fix to master (and so keep the sandbox clean to get feedback about 2.0.0-M7). This will also allow to eventually release a 2.0.0-M8 with only small changes/bug fixes.

@Michal-Wadowski, @rikard-sics does it sounds good to you ?

rikard-sics commented 2 years ago
  • OSCORE minimal viable feature. (Let me know if there is more than that ?)

@Michal-Wadowski, @rikard-sics does it sounds good to you ?

Yes, I think that sounds like a good plan.

Michal-Wadowski commented 2 years ago

@Michal-Wadowski, @rikard-sics does it sounds good to you ?

Yes, it looks ok for me too :slightly_smiling_face:

sbernard31 commented 2 years ago

Hi all,

I have some concerns about the current plan. I think I should really move forward concerning #1025 and so we need maybe a less ambitious plan.

(Above I talked about 2.0.0-M7 and 2.0.0-M8 but we deliver an unexpected 2.0.0-M7, so now we are talking about 2.0.0-M8 and 2.0.0-M9)

As I said above, #1025 will be a huge refactoring. During I work on this, I think we should limit as much as possible to integrate feature in master.

Contributors could still work on their side but they should keep in mind :

sbernard31 commented 2 years ago

:heavy_check_mark: 1. is done (https://github.com/eclipse/leshan/issues/1265#issuecomment-1176250115)

I will work on 2. (trying to integrate oscore branch in master)

sbernard31 commented 2 years ago

I tried to investigate on #1257 without success :disappointed: (https://github.com/eclipse/leshan/issues/1257#issuecomment-1178988829) So I integrated oscore in master (#1277) as planned, meaning : :heavy_check_mark: 2. is done.

I will now work on 3. (formatter and organize import from https://github.com/eclipse/leshan/issues/1265)

rikard-sics commented 2 years ago

I tried to investigate on #1257 without success disappointed (#1257 (comment))

Thanks for looking into this. I will take some time to investigate this also, still good to figure out what is going wrong in that scenario.

sbernard31 commented 2 years ago

:heavy_check_mark: 3. is done with :

I will be unavailable next days back on Monday.

@rikard-sics if you find a solution for #1257 for Monday this would be great, this way I could release the 2.0.0-M8 and so finally start to work on #1025.

Even if you didn't finish, do not hesitate to share current state of your investigation at #1257. :pray:

sbernard31 commented 2 years ago

@rikard-sics, @adamsero, @JaroslawLegierski.

Please let me know if we need to add some missing stuff for 2.0.0-M8.

If I don't get any news about this, I will release the 2.0.0-M8 on Wednesday, and then I will enter in a code freeze period during my work on #1025.

sbernard31 commented 2 years ago

@rikard-sics, @adamsero, @JaroslawLegierski.

I prepare the release today and release it publicly tomorrow in the morning. So you have until tomorrow morning 7.00 pm to eventually raise a No Go. :slightly_smiling_face:

sbernard31 commented 2 years ago

I released the 2.0.0-M8

So now I will full focus on #1025. I plan to not integrate more feature in master during this period. Integrating small bug fixes in master is OK.

sbernard31 commented 2 years ago

(Just to let you know guys, I will be unavailable until 22th August)

sbernard31 commented 1 year ago

@adamsero, I started to work on #1025.

Experiment some ideas at #1220 and #1312 : you could have a look at this but this is a crappy POC and maybe better to wait for a cleaner version.

I also propose a new module design at #1295 : feedback about this is welcome :pray:

I'm currently working on a cleaner version for abstraction at server side. When this will be ready feedback will be welcome too.

Once we will have a cleaner version of the abstraction, I see some task that you could eventually work on :

  1. create another transport layer for coap (eventually coaps too) with another java library than californium, I guess this should not be very clean code. I don't think we will release it. This is just a try to be sure the LWM2M endpoint abstraction is good enough. Do you see the idea ?
  2. Maybe we should rewrite some tests to make possible to reuse same test for different transport layer ? (just an idea, I have not clear idea about this)

At https://github.com/eclipse/leshan/issues/1049#issuecomment-923916916, there is idea about supporting NIDD this could also be a very good way to experiment test the LWM2M endpoint abstraction.

Globally the idea is giving feedback about the design and find issue/bug/regression about it.

sbernard31 commented 1 year ago

About "create another transport layer for coap", I created a issue #1338

sbernard31 commented 1 year ago

Experiment some ideas at https://github.com/eclipse/leshan/pull/1220 and https://github.com/eclipse/leshan/pull/1312 : you could have a look at this but this is a crappy POC and maybe better to wait for a cleaner version. I'm currently working on a cleaner version for abstraction at server side. When this will be ready feedback will be welcome too.

The cleaner version are now available at : #1318, #1323 , #1336. Currently, there're missing some polish but unless we find some blocking issue. It should not change too much.

So, missing task about transport layer are :

I'm not sure in which order I should proceed now :thinking: I also don't know, if I should integrate #1318, #1323 , #1336 in master now or if this is too soon.

The issue is that until now, I didn't succeed to get any feedback from community about all of those. So, I'm not hyper confident about all those changes.

@adamsero or any Orange guys , do you have any opinion about that ? Let me know if you plan to work on some of this or if you plan to provide feedback ?

Waiting from you, I think I will try to fix #1314.

JaroslawLegierski commented 1 year ago

I have started work on testing coap-java with Leshan. In the first stage of learning coap-java, I registered example-client example-client in Leshan server. In the next step, I would like to replace californium with the coap-java library (probably on the client side first).

sbernard31 commented 1 year ago

@JaroslawLegierski good news. I hope you based your work on #1323 ?

Do not hesitate to create an issue to discuss about it or just to give some news regularly about this topic. Do not hesitate to contact the java-coap maintainer (szysas), he says me he is OK to help.

JaroslawLegierski commented 1 year ago

Yes I'm using endpoint_client branch from PR #1323

sbernard31 commented 1 year ago

@JaroslawLegierski , @adamsero, @Warmek,

I would like to propose a plan. I think we could consider to release a 2.0.0-M10 without transport layer (#1025) but with :

Then after this 2.0.0-M10, I integrate #1318, #1323 , #1336 in master. This way we can get feedback from Leshan demo at the beginning of next milestone development phase.

Does it sound good to you ?

JaroslawLegierski commented 1 year ago

I think that this is very good idea I would like add one more point:

Today we also identified one problem with Californium congestion control but I haven't analyzed this problem yet and I don't know if it concerns Leshan or Cf ? (I will test it tomorrow or the day after tomorrow)

How do you think it will be possible to release the M10 by mid-December?

Regarding test of #1323 I asked szysas for help but I haven't got an answer from him yet

sbernard31 commented 1 year ago

Then after this 2.0.0-M10, plan to integrate https://github.com/eclipse/leshan/pull/1318, https://github.com/eclipse/leshan/pull/1323 , https://github.com/eclipse/leshan/pull/1336 in master. This way we can get feedback from Leshan demo at the beginning of next milestone development phase.

@JaroslawLegierski do still you agree with this idea ? This will impact a lot Leshan code base with several API break and even Redis Store Serialization format will be broken. We will enter in phase where Leshan will be less stable in term of API but also maybe we will face some regression.

I know this not so encouraging but I guess at some time we have to take the plunge.

Do you know if at Orange someone test to integrate this PR in a product or a tool based on Leshan ?

JaroslawLegierski commented 1 year ago

Sorry for the late reply. In general, the plan is OK for us, however we would like to address following topics (conditions):

  1. No need at our side to use something else than Californium.
  2. Need that Leshan is able (just in case of) to release fixes on M10 before releasing M11 - How do you see work on fixes for the 2.0.0-M10 release (e.g. in case of a serious bug) in a situation where version 2.0.0-M11 will not be mature enough for production use yet?
    1. We will be able to integrate a Master Branch M11 candidate and perform some unit tests within our product.

I have one important question from our French colleagues: When should we test M11 candidate ?

sbernard31 commented 1 year ago

Sorry for the late reply.

No problem. :slightly_smiling_face:

  1. No need at our side to use something else than Californium.

Yep this is still planned to provide transport layer based on Californium for coap and coaps. For coap+tcp, there is less clear.

  1. Need that Leshan is able (just in case of) to release fixes on M10 before releasing M11 - How do you see work on fixes for the 2.0.0-M10 release (e.g. in case of a serious bug) in a situation where version 2.0.0-M11 will not be mature enough for production use yet?

I hope we will be able to avoid this. Either by finding some workaround for M10 to help you to fix your bugs if you don't want to jump to M11 too soon. Or by succeeding to make M11 stable fast enough. In all case, if no ways above works, we will find a way to release something with a fix and without transport layer. (I'm not worry about it)

  1. We will be able to integrate a Master Branch M11 candidate and perform some unit tests within our product. When should we test M11 candidate ?

The most often you can test and the sooner you can give feedback, the better it is. (any kind of feedback : bugs, remarks about API, design, readability, missing features ... ... )

If you want to test now you can just take content of branch endpoint_bootstrap. If this is OK for you and as I will be unavailable next week, I plan to rather integrate this branch in master when I'm back. So if someone face bug or need support, I will be able to answer quickly.

About discussion with your french colleagues, if wanted they can also participate to conversation here (or any conversation about Leshan) , everybody is welcome :)

sbernard31 commented 1 year ago

@JaroslawLegierski, let me know if this sounds good for you.

@jvermillard same for you and by the way if you can test the endpoint_bootstrap branch, this is maybe a good idea to do it before to integrate it in master ?

Happy holidays, I'll be back in 2023 :wink: !

sbernard31 commented 1 year ago

I'm back in business. I hope you guys had good holiday.

@JaroslawLegierski following my answers (https://github.com/eclipse/leshan/issues/1222#issuecomment-1361372075), I'm waiting for your green light before to integrate transport layer abstraction into master.

@jvermillard did you try (or do you plan) to integrate endpoint_bootstrap branch and launch your integration tests ? (I would like to know if I should wait for it before the integration in master)

jvermillard commented 1 year ago

@sbernard31 nop, cause I run on top of the obj25 branch, which is based on the master and at the moment I have little testing for my bootstrap process

sbernard31 commented 1 year ago

cause I run on top of the obj25 branch, which is based on the master and at the moment I have little testing for my bootstrap process

Just to be sure, there isn't miss-understanding, the idea is to make endpoint_bootstrap branch the new master soon and endpoint_bootstrap branch contains transport layer abstraction for client, server and bootstrap server.

JaroslawLegierski commented 1 year ago

Your explanation is OK for me. I also sent your comments to my french colleagues (along with the invitation to discuss here :-) ).

sbernard31 commented 1 year ago

@JaroslawLegierski OK, then I wait for their green light :slightly_smiling_face:

jvermillard commented 1 year ago

I rebased the obj25 support on it and tested it with my lwm2m implementation integration test suite and it works

JaroslawLegierski commented 1 year ago

@JaroslawLegierski OK, then I wait for their green light 🙂

I just got a reply from colleagues - we are OK with your answers. Therefore You have green light to integrate transport layer abstraction into master.

sbernard31 commented 1 year ago

First version of transport abstraction layer is now integrated in master.

See https://github.com/eclipse/leshan/issues/1025#issuecomment-1373824399 for more details.