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

Organization for refactoring about transport layer #1222

Open
sbernard31 opened this issue Mar 16, 2022 · 31 comments
Open

Organization for refactoring about transport layer #1222

sbernard31 opened this issue Mar 16, 2022 · 31 comments
Labels
discussion Discussion about anything

Comments

@sbernard31
Copy link
Contributor

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 sbernard31 added the discussion Discussion about anything label Mar 16, 2022
@rikard-sics
Copy link
Contributor

  • 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
Copy link
Contributor

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

Yes, it looks ok for me too 🙂

@sbernard31
Copy link
Contributor Author

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 Enhance Continious Integration Process. #1265 (@Michal-Wadowski 's idea 😉 )
  2. I will integrate current OSCORE branch in master even if there is known issue about "common" use case : OSCORE fallback use cases #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 ☝️ (before the 2.0.0-M8 release) :

  • @rikard-sics ideally if you have time could you work on OSCORE fallback use cases #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
Copy link
Contributor Author

sbernard31 commented Jul 6, 2022

✔️ 1. is done (#1265 (comment))

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

@sbernard31
Copy link
Contributor Author

I tried to investigate on #1257 without success 😞 (#1257 (comment))
So I integrated oscore in master (#1277) as planned, meaning :
✔️ 2. is done.

I will now work on 3. (formatter and organize import from #1265)

@rikard-sics
Copy link
Contributor

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
Copy link
Contributor Author

✔️ 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. 🙏

@sbernard31
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@sbernard31
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@sbernard31
Copy link
Contributor Author

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

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 #1049 (comment), 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
Copy link
Contributor Author

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

@sbernard31
Copy link
Contributor Author

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'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 🤔
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
Copy link
Contributor

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
Copy link
Contributor Author

@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
Copy link
Contributor

Yes I'm using endpoint_client branch from PR #1323

@sbernard31
Copy link
Contributor Author

sbernard31 commented Nov 25, 2022

@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
Copy link
Contributor

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 sbernard31 mentioned this issue Dec 8, 2022
3 tasks
@sbernard31
Copy link
Contributor Author

Then after this 2.0.0-M10, plan to integrate #1318, #1323 , #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
Copy link
Contributor

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 ?

@sbernard31
Copy link
Contributor Author

Sorry for the late reply.

No problem. 🙂

  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
Copy link
Contributor Author

sbernard31 commented Dec 22, 2022

@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 😉 !

@sbernard31
Copy link
Contributor Author

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

@JaroslawLegierski following my answers (#1222 (comment)), 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
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor

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

@sbernard31
Copy link
Contributor Author

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

@jvermillard
Copy link
Contributor

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

@JaroslawLegierski
Copy link
Contributor

@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
Copy link
Contributor Author

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

See #1025 (comment) for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion about anything
Projects
None yet
Development

No branches or pull requests

5 participants