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

Feature eth v9 #31

Merged
merged 28 commits into from
Apr 16, 2024
Merged

Feature eth v9 #31

merged 28 commits into from
Apr 16, 2024

Conversation

pkazanzides
Copy link
Contributor

Changes for Ethernet-only network (FPGA V3), with option to force use of Ethernet/Firewire bridge.

@pkazanzides pkazanzides requested a review from adeguet1 February 15, 2024 01:02
@adeguet1
Copy link
Contributor

I'm fine with the naming choice, i.e. fw, udp, udpfw, eth, ethfw. The main issue is the change of meaning for 'udp' breaking backward compatibility. There are not many groups actively using udp so changing its meaning won't be too disruptive. But it might still be a bit confusing for v2 users. Let's say I'm using a single arm and connecting with ethernet. It would make sense to use udp since it's not obvious there's a fw bus in the v2 controller. Maybe we can create udpfw, udpudp and fwfw while deprecating udp and fw None of these are pretty though.

@pkazanzides
Copy link
Contributor Author

Yes, I thought about this for some time. My conclusion was that "udp" (or "eth") means to use the default (or best) setup for the given hardware, which would be a backward-compatible view. For FPGA V2, it always means using the Ethernet/Firewire bridge (i.e., same as "udpfw" or "ethfw"). Another way to look at this is that "udpfw" and "ethfw" are only relevant for FPGA V3, in which case there is no backward compatibility issue. Maybe it wasn't obvious in the code that the software would still use Firewire if you specify "udp" on a V2 system.

@pkazanzides pkazanzides merged commit cb9b0c6 into devel Apr 16, 2024
6 checks passed
@pkazanzides pkazanzides deleted the feature-eth-v9 branch April 16, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants