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

Make new directory under federated named network #292

Merged
merged 32 commits into from
Dec 2, 2023
Merged

Conversation

Jakio815
Copy link
Collaborator

@Jakio815 Jakio815 commented Oct 21, 2023

This PR is the first step abstracting the network layer which is in #281.

This changes the file structure of the reactor-c, adding a new directory under federated named networks.

Currently only moved net_util.c net_common.h and net_util.h under newly named directory networks.

This is to add more files in future such as net_tls.c.

The corresponding CMakelists.txt files have been modified together.

The idea comes from how the platform works. Each platform related header such as lf_arduino_support.h lf_rp2040_support.h is inside the directory platform.

This must be merged together with lf-lang/lingua-franca#2070

@Jakio815
Copy link
Collaborator Author

@lhstrh This was discussed in the security meeting. The point is just moving network related c and header files under a directory network. Do you think it's fine?

@lhstrh
Copy link
Member

lhstrh commented Oct 25, 2023

I would call network or net, not networks, but the idea seems fine. Let me request review from @erlingrj.

@lhstrh lhstrh requested a review from erlingrj October 25, 2023 19:39
@erlingrj
Copy link
Collaborator

Looks like a reasonable approach. Just one question about the new CMakeLists.txt

@Jakio815
Copy link
Collaborator Author

I would call network or net, not networks, but the idea seems fine. Let me request review from @erlingrj.

Fixed all to network

@Jakio815 Jakio815 marked this pull request as ready for review November 6, 2023 19:51
@Jakio815 Jakio815 changed the title Draft: Make new directory under federated named networks. Make new directory under federated named networks. Nov 6, 2023
@Jakio815 Jakio815 requested a review from lhstrh November 6, 2023 19:52
@Jakio815
Copy link
Collaborator Author

Jakio815 commented Dec 1, 2023

@lhstrh Can I merge this together with lf-lang/lingua-franca#2070?

@lhstrh
Copy link
Member

lhstrh commented Dec 1, 2023

I noticed that not all test in lf-lang/lingua-franca#2070 are passing though...

@lhstrh lhstrh changed the title Make new directory under federated named networks. Make new directory under federated named network Dec 1, 2023
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

Looks good modulo comment.

lingua-franca-ref.txt Outdated Show resolved Hide resolved
@Jakio815
Copy link
Collaborator Author

Jakio815 commented Dec 2, 2023

@lhstrh Oops. Sorry for that. I thought it was finished. The lingua-franca's check is finished now, but I see that the reactor-c is being kept updated by Professor Lee. Should I merge it after his update is finished?

@lhstrh
Copy link
Member

lhstrh commented Dec 2, 2023

I think it's OK to just go ahead and merge once the tests pass.

@edwardalee
Copy link
Contributor

As long as you are using reactor-c/main it is OK to merge this.

@Jakio815 Jakio815 merged commit b32b39d into main Dec 2, 2023
28 checks passed
@Jakio815 Jakio815 deleted the network-structure branch December 2, 2023 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants