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

Improve network management #1478

Merged

Conversation

crschardt
Copy link
Contributor

@crschardt crschardt commented Oct 20, 2024

This PR changes the way that photonvision interacts with nmcli to control networking on the coprocessor. Instead of modifying an existing connection, Photonvision adds new connections for DHCP and Static IP configurations. It then activiates the proper one at startup and any time that the network configuration is changed.

It also now uses the interface name and not the connection name and it checks that the interface is available before making any changes. If the saved interface is not found, it updates the stored interface name and applies the network settings to the current interface. This should minimize the failure to control the network if the network interface wasn't available when PhotonVision first booted.

One other benefit of not altering the default configuration is that, if PhotonVision fails to run for any reason, the device can be accessed using the original networking configuration.

I've tested this on an OrangePi and so far, it is working very well.

Addresses: #1261

This commit changes the way that photonvision interacts with nmcli
to control networking on the coprocessor. Instead of modifying an
existing connection, Photonvision adds new connections for DHCP and
Static IP configurations. It then activiates the proper one at
startup and any time that the network configuration is changed. It
also now uses the interface name and not the connection name and
it checks that the interface is available before making any changes.
If the saved interface is not found, it updates the stored interface
name and applies the network settings to the current interface. This
should minimize the failure to control the network if the network
interface wasn't available when PhotonVision first booted.
@crschardt crschardt requested a review from a team as a code owner October 20, 2024 02:29
Copy link
Contributor

@mcm001 mcm001 left a comment

Choose a reason for hiding this comment

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

High level this looks good to me -- can test locally as well

Copy link
Contributor

@Juniormunk Juniormunk left a comment

Choose a reason for hiding this comment

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

Works good for me!

@crschardt crschardt merged commit 8ff0d93 into PhotonVision:master Oct 21, 2024
31 checks passed
@crschardt
Copy link
Contributor Author

I added active monitoring of wired connections. It now logs when an ethernet port is disconnected and it re-initializes the network connection when it is re-connected. This should ensure that the configuration is the one that is set up in PhotonVision and not the one that NetworkManager decides to apply. It also means that, if the device is disconnected when it boots, but is then connected, it will have the right configuration applied.

@crschardt crschardt deleted the improve-network-connection-management branch October 21, 2024 03:24
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.

3 participants