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.

Sign up for GitHub

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

[tui] Implement #135 for update #246

Merged
merged 6 commits into from
Jan 20, 2025
Merged

[tui] Implement #135 for update #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6373f58
[tui] Implement #135 for update
goldyfruit Jan 18, 2025
e6661ba
[tui] Implement #135 for update
goldyfruit Jan 18, 2025
209df2f
[tui] Implement #135 for update
goldyfruit Jan 20, 2025
ae8d1b3
[tui] Implement #135 for update
goldyfruit Jan 20, 2025
92fd641
[tui] Implement #135 for update
goldyfruit Jan 20, 2025
d4d1d3c
[tui] Implement #135 for update
goldyfruit Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ansible/roles/ovos_installer/tasks/ovos.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- "{{ ovos_installer_user_home }}/.config/OpenVoiceOS/*"
- "{{ ovos_installer_user_home }}/.config/wireplumber/*"
- "{{ ovos_installer_user_home }}/.local/share/hivemind/*"
- "{{ ovos_installer_user_home }}/.local/state/ovos/*"
when: ovos_installer_cleaning | bool
tags:
- uninstall
Expand Down
1 change: 1 addition & 0 deletions ansible/roles/ovos_installer/tasks/virtualenv/systemd.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,5 +135,6 @@
- "{{ ovos_installer_user_home }}/.local/share/precise-lite"
- "{{ ovos_installer_user_home }}/.local/state/mycroft"
- "{{ ovos_installer_user_home }}/.local/state/vosk"
- "{{ ovos_installer_user_home }}/.local/state/ovos"
- "{{ ovos_installer_user_home }}/nltk_data"
- "{{ ovos_installer_user_home }}/stdout"
1 change: 1 addition & 0 deletions setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ install_ansible
download_yq
detect_scenario
i2c_scan
state_directory
trap "" ERR
set +eE

Expand Down
29 changes: 29 additions & 0 deletions tui/features.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,31 @@ export FEATURE_GUI="false"
export FEATURE_SKILLS="false"
export FEATURE_EXTRA_SKILLS="false"

declare -a features
features=("skills" "$SKILL_DESCRIPTION" ON)
features+=("extra-skills" "$EXTRA_SKILL_DESCRIPTION" OFF)
if [[ "$RASPBERRYPI_MODEL" != *"Raspberry Pi 3"* ]] && [[ "$KERNEL" != *"microsoft"* ]] && [ "$PROFILE" != "server" ]; then
features+=("gui" "$GUI_DESCRIPTION" OFF)
fi

if [ -f "$INSTALLER_STATE_FILE" ]; then
if jq -e '.features|any(. == "skills")' "$INSTALLER_STATE_FILE" &>>"$LOG_FILE"; then
features=("skills" "$SKILL_DESCRIPTION" ON)
else
features=("skills" "$SKILL_DESCRIPTION" OFF)
fi
if jq -e '.features|any(. == "extra-skills")' "$INSTALLER_STATE_FILE" &>>"$LOG_FILE"; then
features+=("extra-skills" "$EXTRA_SKILL_DESCRIPTION" ON)
else
features+=("extra-skills" "$EXTRA_SKILL_DESCRIPTION" OFF)
fi
if jq -e '.features|any(. == "gui")' "$INSTALLER_STATE_FILE" &>>"$LOG_FILE"; then
features+=("gui" "$GUI_DESCRIPTION" ON)
else
features+=("gui" "$GUI_DESCRIPTION" OFF)
fi
fi

OVOS_FEATURES=$(whiptail --separate-output --title "$TITLE" \
--checklist "$CONTENT" --cancel-button "$BACK_BUTTON" --ok-button "$OK_BUTTON" --yes-button "$OK_BUTTON" "$TUI_WINDOW_HEIGHT" "$TUI_WINDOW_WIDTH" "${#features[@]}" "${features[@]}" 3>&1 1>&2 2>&3)

Expand All @@ -27,16 +46,26 @@ if [ "$exit_status" -ne 0 ]; then
fi
fi

declare -a FEATURES_STATE
for FEATURE in $OVOS_FEATURES; do
case "$FEATURE" in
"gui")
export FEATURE_GUI="true"
FEATURES_STATE+=("gui")
;;
"skills")
export FEATURE_SKILLS="true"
FEATURES_STATE+=("skills")
;;
"extra-skills")
export FEATURE_EXTRA_SKILLS="true"
FEATURES_STATE+=("extra-skills")
;;
esac
done

if [ "$exit_status" -ne 1 ]; then
jq -en '.features += $ARGS.positional' --args "${FEATURES_STATE[@]}" > "$TEMP_FEATURES_FILE"
jq -es '.[0] * .[1]' "$TEMP_PROFILE_FILE" "$TEMP_FEATURES_FILE" > "$INSTALLER_STATE_FILE"
rm "$TEMP_FEATURES_FILE" "$TEMP_PROFILE_FILE"
fi
Comment on lines +67 to +71
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve temporary file handling and cleanup.

The script needs better error handling and cleanup for temporary files.

Add proper cleanup and error handling:

+cleanup() {
+  rm -f "$TEMP_FEATURES_FILE" "$TEMP_PROFILE_FILE"
+}
+trap cleanup EXIT
+
 if [ "$exit_status" -ne 1 ]; then
-  jq -en '.features += $ARGS.positional' --args "${FEATURES_STATE[@]}" > "$TEMP_FEATURES_FILE"
-  jq -es '.[0] * .[1]' "$TEMP_PROFILE_FILE" "$TEMP_FEATURES_FILE" > "$INSTALLER_STATE_FILE"
-  rm "$TEMP_FEATURES_FILE" "$TEMP_PROFILE_FILE"
+  jq -en '.features += $ARGS.positional' --args "${FEATURES_STATE[@]}" > "$TEMP_FEATURES_FILE" || {
+    echo "Error: Failed to create features file" >&2
+    exit 1
+  }
+  jq -es '.[0] * .[1]' "$TEMP_PROFILE_FILE" "$TEMP_FEATURES_FILE" > "$INSTALLER_STATE_FILE" || {
+    echo "Error: Failed to merge state files" >&2
+    exit 1
+  }
 fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "$exit_status" -ne 1 ]; then
jq -en '.features += $ARGS.positional' --args "${FEATURES_STATE[@]}" > "$TEMP_FEATURES_FILE"
jq -es '.[0] * .[1]' "$TEMP_PROFILE_FILE" "$TEMP_FEATURES_FILE" > "$INSTALLER_STATE_FILE"
rm "$TEMP_FEATURES_FILE" "$TEMP_PROFILE_FILE"
fi
cleanup() {
rm -f "$TEMP_FEATURES_FILE" "$TEMP_PROFILE_FILE"
}
trap cleanup EXIT
if [ "$exit_status" -ne 1 ]; then
jq -en '.features += $ARGS.positional' --args "${FEATURES_STATE[@]}" > "$TEMP_FEATURES_FILE" || {
echo "Error: Failed to create features file" >&2
exit 1
}
jq -es '.[0] * .[1]' "$TEMP_PROFILE_FILE" "$TEMP_FEATURES_FILE" > "$INSTALLER_STATE_FILE" || {
echo "Error: Failed to merge state files" >&2
exit 1
}
fi

14 changes: 14 additions & 0 deletions tui/locales/en-us/update.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/env bash

CONTENT="
An existing instance of Open Voice OS/HiveMind has been detected.
Upgrading your existing instance of Open Voice OS or HiveMind to the latest version ensures that you stay ahead with the most advanced features, critical security updates, bug fixes, and performance optimizations.
This not only enhances the overall functionality and user experience but also ensures compatibility with the latest integrations and tools, keeping your system reliable, secure, and future-proof.
Do you want to update Open Voice OS to the latest version?
"
TITLE="Open Voice OS Installation - Update"

export CONTENT TITLE
13 changes: 13 additions & 0 deletions tui/profiles.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,19 @@
# shellcheck source=locales/en-us/profiles.sh
source "tui/locales/$LOCALE/profiles.sh"

# Default active and available profiles
active_profile="ovos"
available_profiles=(ovos satellite listener server)

# Handle existing installation
if [ -f "$INSTALLER_STATE_FILE" ]; then
if jq -e 'has("profile")' "$INSTALLER_STATE_FILE" &>>"$LOG_FILE"; then
current_profile=$(jq -re '.profile' "$INSTALLER_STATE_FILE")
active_profile="$current_profile"
available_profiles=("$current_profile")
fi
Comment on lines +11 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for JSON operations.

The script uses jq without proper error handling, which could lead to silent failures.

Add error handling:

 if [ -f "$INSTALLER_STATE_FILE" ]; then
-  if jq -e 'has("profile")' "$INSTALLER_STATE_FILE" &>>"$LOG_FILE"; then
-    current_profile=$(jq -re '.profile' "$INSTALLER_STATE_FILE")
+  if ! jq -e 'has("profile")' "$INSTALLER_STATE_FILE" &>>"$LOG_FILE"; then
+    echo "Error: Invalid state file format" >&2
+    exit 1
+  fi
+  current_profile=$(jq -re '.profile' "$INSTALLER_STATE_FILE") || {
+    echo "Error: Failed to read profile from state file" >&2
+    exit 1
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ -f "$INSTALLER_STATE_FILE" ]; then
if jq -e 'has("profile")' "$INSTALLER_STATE_FILE" &>>"$LOG_FILE"; then
current_profile=$(jq -re '.profile' "$INSTALLER_STATE_FILE")
active_profile="$current_profile"
available_profiles=("$current_profile")
fi
if [ -f "$INSTALLER_STATE_FILE" ]; then
if ! jq -e 'has("profile")' "$INSTALLER_STATE_FILE" &>>"$LOG_FILE"; then
echo "Error: Invalid state file format" >&2
exit 1
fi
current_profile=$(jq -re '.profile' "$INSTALLER_STATE_FILE") || {
echo "Error: Failed to read profile from state file" >&2
exit 1
}
active_profile="$current_profile"
available_profiles=("$current_profile")
fi

fi

whiptail_args=(
--title "$TITLE"
--radiolist "$CONTENT"
Expand All @@ -31,3 +41,6 @@ if [ -z "$PROFILE" ]; then
source tui/channels.sh
source tui/profiles.sh
fi

jq -en --arg profile "$PROFILE" '.profile += $profile' > "$TEMP_PROFILE_FILE"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential race conditions with temporary files.

The script writes to a temporary file without proper locking mechanisms.

Add file locking:

-jq -en --arg profile "$PROFILE" '.profile += $profile' > "$TEMP_PROFILE_FILE"
+(
+  flock -w 10 200 || {
+    echo "Error: Failed to acquire lock" >&2
+    exit 1
+  }
+  jq -en --arg profile "$PROFILE" '.profile += $profile' > "$TEMP_PROFILE_FILE"
+) 200>"${TEMP_PROFILE_FILE}.lock"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
jq -en --arg profile "$PROFILE" '.profile += $profile' > "$TEMP_PROFILE_FILE"
(
flock -w 10 200 || {
echo "Error: Failed to acquire lock" >&2
exit 1
}
jq -en --arg profile "$PROFILE" '.profile += $profile' > "$TEMP_PROFILE_FILE"
) 200>"${TEMP_PROFILE_FILE}.lock"


3 changes: 3 additions & 0 deletions tui/uninstall.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,7 @@ whiptail --yesno --defaultno --no-button "$NO_BUTTON" --yes-button "$YES_BUTTON"
exit_status=$?
if [ "$exit_status" -eq 1 ]; then
export CONFIRM_UNINSTALL="false"

# shellcheck source=update.sh
source tui/update.sh
Comment on lines +17 to +18
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling and improve user flow clarity.

When uninstall is declined, the script directly sources update.sh without error handling or user notification.

Add error handling and user notification:

-  # shellcheck source=update.sh
-  source tui/update.sh
+  # shellcheck source=update.sh
+  if [ ! -f "tui/update.sh" ]; then
+    echo "Error: Update script not found" >&2
+    exit 1
+  fi
+  whiptail --msgbox --title "$TITLE" "Proceeding to update menu..." \
+    "$TUI_WINDOW_HEIGHT" "$TUI_WINDOW_WIDTH"
+  source tui/update.sh
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# shellcheck source=update.sh
source tui/update.sh
# shellcheck source=update.sh
if [ ! -f "tui/update.sh" ]; then
echo "Error: Update script not found" >&2
exit 1
fi
whiptail --msgbox --title "$TITLE" "Proceeding to update menu..." \
"$TUI_WINDOW_HEIGHT" "$TUI_WINDOW_WIDTH"
source tui/update.sh

fi
14 changes: 14 additions & 0 deletions tui/update.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/env bash

# shellcheck source=locales/en-us/misc.sh
source "tui/locales/$LOCALE/misc.sh"
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for locale file sourcing.

The script assumes locale files exist but doesn't handle cases where they might be missing.

Add error checking:

-source "tui/locales/$LOCALE/misc.sh"
+if [ ! -f "tui/locales/$LOCALE/misc.sh" ]; then
+  echo "Error: Missing locale file tui/locales/$LOCALE/misc.sh" >&2
+  exit 1
+fi
+source "tui/locales/$LOCALE/misc.sh"

Also applies to: 7-7


# shellcheck source=locales/en-us/update.sh
source "tui/locales/$LOCALE/update.sh"

whiptail --yesno --no-button "$NO_BUTTON" --yes-button "$YES_BUTTON" --title "$TITLE" "$CONTENT" "$TUI_WINDOW_HEIGHT" "$TUI_WINDOW_WIDTH"

exit_status=$?
if [ "$exit_status" -eq 1 ]; then
exit 0
fi
15 changes: 15 additions & 0 deletions utils/common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,3 +548,18 @@ function apt_ensure() {
echo "No missing packages"
fi
}

# This function ensures the existence and proper configuration of a
# local state directory for the OVOS environment. It sets up a
# specific directory structure and prepares an installer state file for use.
function state_directory() {
OVOS_LOCAL_STATE_DIRECTORY="$RUN_AS_HOME/.local/state/ovos"
export INSTALLER_STATE_FILE="$OVOS_LOCAL_STATE_DIRECTORY/installer.json"
if [ ! -d "$OVOS_LOCAL_STATE_DIRECTORY" ]; then
mkdir -p "$OVOS_LOCAL_STATE_DIRECTORY" &>>"$LOG_FILE"
chown -R "$RUN_AS":"$(id -ng "$RUN_AS")" "$OVOS_LOCAL_STATE_DIRECTORY" &>>"$LOG_FILE"
fi
if [ -f "$INSTALLER_STATE_FILE" ]; then
[ -s "$INSTALLER_STATE_FILE" ] || rm "$INSTALLER_STATE_FILE" &>>"$LOG_FILE"
fi
}
2 changes: 2 additions & 0 deletions utils/constants.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ declare -rA SUPPORTED_DEVICES=(
["tas5806"]="2f" #https://www.ti.com/product/TAS5806MD
)
export SUPPORTED_DEVICES
export TEMP_FEATURES_FILE=/tmp/features.json
export TEMP_PROFILE_FILE=/tmp/profile.json
Comment on lines +57 to +58
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use secure temporary file paths.

Using hardcoded paths in /tmp could lead to security issues in multi-user environments.

Use mktemp for secure temporary files:

-export TEMP_FEATURES_FILE=/tmp/features.json
-export TEMP_PROFILE_FILE=/tmp/profile.json
+export TEMP_FEATURES_FILE
+export TEMP_PROFILE_FILE
+TEMP_FEATURES_FILE=$(mktemp -t ovos-features.XXXXXX.json)
+TEMP_PROFILE_FILE=$(mktemp -t ovos-profile.XXXXXX.json)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export TEMP_FEATURES_FILE=/tmp/features.json
export TEMP_PROFILE_FILE=/tmp/profile.json
export TEMP_FEATURES_FILE
export TEMP_PROFILE_FILE
TEMP_FEATURES_FILE=$(mktemp -t ovos-features.XXXXXX.json)
TEMP_PROFILE_FILE=$(mktemp -t ovos-profile.XXXXXX.json)

export TUI_WINDOW_HEIGHT="35"
export TUI_WINDOW_WIDTH="90"
export USER_ID="$EUID"
Expand Down
Loading