search

GiPHouse / qbaylogic-clash-based-macipudp-stack-spring24

A fully configurable Ethernet core written in Clash.
Other
5 stars 2 forks source link

Upstream PacketStream to Clash.Protocols #122

Open rowanG077 opened 6 months ago

rowanG077 commented 6 months ago

Feature description Beside the ethernet stack qbaylogic would really like to have the PacketStream protocol inside Clash.Protocols. This issue is to upstream the protocol + all our generic modules.

Expected behaviour PR to Clash.Protocols is made and accepted.

Expected steps Clone the clash protocols repo and move our PacketStream protocol to it and make a PR.

Additional Context

MatthijsMu commented 5 months ago

Just PacketStream, or also components such as up/downconverter, (de)packetizer(to/from Df)?

rowanG077 commented 5 months ago

the protocol + all our generic modules.

MatthijsMu commented 5 months ago

I can imagine the up+downconverter, de+packetizer want to be merged into one file (perhaps also with Df), but it may be useless to do this prematurely. @lmbollen could you tell me more about the filestructure that you would like to see?

rowanG077 commented 5 months ago

I would not merge everything into Df, definitely a separate module. Theoretically we could merge it into one file but personally I would like to keep at least a bit split up. Mainly because a single HS module is compiled sequentially whereas multiple HS files can be compiled in parallel.

I would first wait on what @JLaumen comes back with, he had a good proposal in structure he is implementing now.

rowanG077 commented 5 months ago

Also note that this issue is currently not in the sprint. So the issues that are open which are in the sprint have higher priority.

lmbollen commented 5 months ago

I can imagine the up+downconverter, de+packetizer want to be merged into one file (perhaps also with Df), but it may be useless to do this prematurely. @lmbollen could you tell me more about the filestructure that you would like to see?

When there's a pull request, we'll give feedback. Don't worry too much about it for now :)

JLaumen commented 5 months ago

@MatthijsMu The relevant modules have been moved to Protocols.Extra.Packetstream, see #139 (branch of issue #115 )

MatthijsMu commented 5 months ago

oh, that's unfortunate I already did work on this yesterday. Should it remain in Protocols.Extra.Packetstream or should it become Protocols.Packetstream on clash-protocols?

rowanG077 commented 5 months ago

It should become Protocols.Packetstream. The "extra" pattern is done so there will be no namespace clashes with the original. So once it's upstreamed it's no longer extra. But like I said I only want to pick this up once we run out of issues in the sprint.

MatthijsMu commented 5 months ago

okay I'll continue with the ip to eth transformer for now