leshan icon indicating copy to clipboard operation
leshan copied to clipboard

Organization for refactoring about transport layer

Open sbernard31 opened this issue 2 years ago • 31 comments

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 :

  • timestamped node.
  • OSCORE minimal viable feature. (Let me know if there is more than that ?)

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 ?

sbernard31 avatar Mar 16 '22 11:03 sbernard31

  • 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.

rikard-sics avatar Mar 17 '22 09:03 rikard-sics

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

Yes, it looks ok for me too :slightly_smiling_face:

Michal-Wadowski avatar Mar 17 '22 09:03 Michal-Wadowski

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 :

  • that the API will probably change a lot.
  • I will not integrate big feature in master during this time.

So what the new plan :

  1. I need to finish some work on CI https://github.com/eclipse/leshan/issues/1265 (@Michal-Wadowski 's idea :wink: )
  2. I will integrate current OSCORE branch in master even if there is known issue about "common" use case : #1257
  3. I will finish CI enhancement about formatting and organize import (I will need to do a full format/ sort import of the whole code base)

Then I will consider to release the 2.0.0-M8, then I will be able to work on transport layer (help will be welcome on this big task of course)

While I'm working on step above :point_up: (before the 2.0.0-M8 release) :

  • @rikard-sics ideally if you have time could you work on #1257, It would be better to have this fixed for the 2.0.0-M8. (but if we don't have it in time, I will release it anyway. I guess this should not be ideal but OK for a new experimental feature)
  • @rikard-sics, @adamsero, if you want to help more you could take some time to test current OSCORE code base in "common/basic" usage. (no obligation of course)

Does it sounds good to you ? Did I miss some important point ? Do you have some feature you absolutely want before the 2.0.0-M8 release ?

sbernard31 avatar Jul 01 '22 09:07 sbernard31

: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 avatar Jul 06 '22 14:07 sbernard31

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)

sbernard31 avatar Jul 08 '22 13:07 sbernard31

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.

rikard-sics avatar Jul 08 '22 13:07 rikard-sics

:heavy_check_mark: 3. is done with :

  • #1281
  • #1282
  • #1285

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 avatar Jul 13 '22 16:07 sbernard31

@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 avatar Jul 18 '22 16:07 sbernard31

@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 avatar Jul 20 '22 14:07 sbernard31

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 avatar Jul 22 '22 08:07 sbernard31

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

sbernard31 avatar Aug 05 '22 14:08 sbernard31

@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 avatar Sep 21 '22 16:09 sbernard31

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

sbernard31 avatar Oct 25 '22 14:10 sbernard31

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 :

  • maybe testing it with another coap library than californium (#1338) ?
  • maybe implement coap over tcp ? (#1047)
  • refactor code to follow new design : (#1295)
  • some polish as explained in :#1318, #1323 , #1336

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.

sbernard31 avatar Nov 15 '22 16:11 sbernard31

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).

JaroslawLegierski avatar Nov 16 '22 07:11 JaroslawLegierski

@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.

sbernard31 avatar Nov 18 '22 17:11 sbernard31

Yes I'm using endpoint_client branch from PR #1323

JaroslawLegierski avatar Nov 18 '22 18:11 JaroslawLegierski

@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 :

  • Bootstrap Authorizer #1359
  • Base64 enhancement #1325
  • Do you have any other features you would like to add ?

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 ?

sbernard31 avatar Nov 25 '22 16:11 sbernard31

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

  • #1350 (already merged in master)

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

JaroslawLegierski avatar Nov 28 '22 17:11 JaroslawLegierski

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 ?

sbernard31 avatar Dec 19 '22 15:12 sbernard31

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?
  3. 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 ?

JaroslawLegierski avatar Dec 21 '22 13:12 JaroslawLegierski

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 avatar Dec 21 '22 14:12 sbernard31

@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 avatar Dec 22 '22 16:12 sbernard31

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)

sbernard31 avatar Jan 04 '23 09:01 sbernard31

@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

jvermillard avatar Jan 04 '23 12:01 jvermillard

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.

sbernard31 avatar Jan 04 '23 14:01 sbernard31

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

JaroslawLegierski avatar Jan 04 '23 14:01 JaroslawLegierski

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

sbernard31 avatar Jan 04 '23 16:01 sbernard31

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

jvermillard avatar Jan 05 '23 10:01 jvermillard

@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.

JaroslawLegierski avatar Jan 05 '23 16:01 JaroslawLegierski