-
Notifications
You must be signed in to change notification settings - Fork 3
Voldigate dev #32
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
base: main
Are you sure you want to change the base?
Voldigate dev #32
Conversation
… if that was the update route
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything major seems to work fine: the update goes off without a hitch, BotUI launches, and the IDE starts up, but there are a few little details that could be cleaned up.
|
|
||
| [Service] | ||
| ExecStart=/home/kipr/wombat-os/configFiles/checkWombatWiredConnect.sh | ||
| ExecStart=/home/kipr/wombat-os/configFiles/checkWombatWiredConnect_temp.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This filename is incorrect. Did you mean /home/kipr/wombat-os/configFiles/checkWombatWiredConnection_temp.sh?
|
|
||
| while true; do | ||
| # Check if Ethernet cable is plugged in | ||
| if ethtool "$ETH_IF" 2>/dev/null | grep -q "Link detected: yes"; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line can be replaced by a simple cat command
if [ "$(cat /sys/class/net/eth0/link_mode)" -eq "1" ]; then| echo "$(date) - Ethernet not detected, bringing down $WIRED_CONN" | ||
|
|
||
| # Bring down wired connection | ||
| nmcli connection down "$WIRED_CONN" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as other issue, this block doesn't seem like it needs to be re-run constantly.
| fi | ||
|
|
||
| # Check every 5 seconds | ||
| sleep 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are already managing this script with systemd and have Restart set to Always, this while loop isn't strictly necessary. You can remove the while loop here and add RestartSec=5 to the systemd unit file above and achieve roughly the same effect while also allowing systemd to handle keepalive logic.
| git \ | ||
| podman \ | ||
| libxcb-doc -y | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can free even more space by removing the newly orphaned packages:
sudo apt autoremove --purge| echo "$(date) - Ethernet detected, bringing up $WIRED_CONN" | ||
|
|
||
| # Bring up wired connection | ||
| nmcli connection up "$WIRED_CONN" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this nmcli command really need to be re-run every 5 seconds? nmcli and systemctl commands are relatively heavy, so re-running them constantly is very inefficient.
I would suggest adding a stamp file somewhere to indicate which state it's in and checking that first to determine if you need to run the rest of this then block.
This PR: