feat(NVIDIA Driver): show support versions - #8

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

source/usr/local/emhttp/plugins/nvidia-driver/include/gpu-support.php (2)

193-199: Inefficient cache check: lspci is executed before checking cache.

The shell command runs unconditionally on line 193, then the cache is checked afterward. This wastes an exec call when cache exists.

♻️ Proposed fix: check cache first

-$lspci_query = shell_exec('lspci -nn 2>/dev/null');
-if (is_file('/tmp/lspci_output.txt')) {
-  $cached_lspci = `@file_get_contents`('/tmp/lspci_output.txt');
-  if ($cached_lspci !== false && trim($cached_lspci) !== '') {
-    $lspci_query = $cached_lspci;
-  }
-}
+$lspci_query = null;
+if (is_file('/tmp/lspci_output.txt')) {
+  $cached_lspci = `@file_get_contents`('/tmp/lspci_output.txt');
+  if ($cached_lspci !== false && trim($cached_lspci) !== '') {
+    $lspci_query = $cached_lspci;
+  }
+}
+if ($lspci_query === null) {
+  $lspci_query = shell_exec('lspci -nn 2>/dev/null');
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/include/gpu-support.php` around
lines 193 - 199, The code currently runs shell_exec('lspci -nn 2>/dev/null')
into $lspci_query before checking the cache file, causing an unnecessary exec
when cached data exists; change the logic so you first check if
'/tmp/lspci_output.txt' exists and read its contents into $lspci_query (using
`@file_get_contents` and trim check), and only call shell_exec('lspci -nn
2>/dev/null') to populate $lspci_query if the cache is missing or empty; keep
using the same variable name $lspci_query and preserve the existing truthiness
checks so downstream code is unaffected.

---

42-82: Consider handling stale cache when remote fetch fails.

When the cache is expired and the remote fetch fails, the code falls through to the local file (line 84). However, if both remote and local file fail, the stale cache (if it exists) could still be useful. Currently, stale cache data is only used within TTL.

This is a minor edge case - the hardcoded defaults provide a reasonable fallback. No change required unless you want maximum resilience.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/include/gpu-support.php` around
lines 42 - 82, When the remote fetch fails, fall back to a stale cache if
available: after the remote read ($remote_json) fails, check for the existence
of $cache_file (and optionally read $cache_time_file) to load $cached_json,
json_decode it and pass it through
nvidia_normalize_architecture_mapping($cached_data, $default); if normalization
returns a non-empty array return it. Keep the existing normal cache write paths
unchanged, and ensure you only use the stale cached data as a fallback when
remote fetch fails and before falling back to local files.

source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page (1)

205-211: Consider clearing selection when all options are disabled.

When incompatibleAll is true, all options are disabled but the currently selected option remains checked (albeit disabled). This is acceptable given the opacity feedback, but you could optionally uncheck it for clarity.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page` around
lines 205 - 211, When all driver options are disabled (e.g., when
incompatibleAll is true) the currently selected input stays checked; update the
logic around the selected variable (the code that does
document.querySelector('input[name="drv_version"]:checked')) to detect if there
are no enabled inputs
(document.querySelector('input[name="drv_version"]:not(:disabled') returns null)
and, in that case, clear the selection by setting selected.checked = false (or
removeAttribute('checked')) so no option remains checked when all are disabled;
keep the existing branch that selects the first enabled option when one exists.

source/usr/local/emhttp/plugins/nvidia-driver/include/card-support.php (1)

282-300: Minor optimization opportunity in fallback logic.

The two loops over $available_versions (lines 283-289 and 293-299) could be combined by collecting versions and filtering afterward. However, this is a minor concern since the array is typically small.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/include/card-support.php`
around lines 282 - 300, The two separate loops over $available_versions in the
'current' branch can be merged: iterate once over $available_versions inside the
elseif ($required_branch === 'current') block, call
nvidia_extract_version($version) and nvidia_driver_major(...) once per element
and collect normalized versions with their majors (e.g., into a temporary
array), then build $valid_versions by filtering that temp array for majors not
equal to 470, 390, or 340; if the resulting $valid_versions is empty, apply a
second filter on the same temp array to select entries where
nvidia_driver_major(...) === (int)$current_branch_major and populate
$valid_versions — update references to nvidia_extract_version,
nvidia_driver_major, $valid_versions, and $current_branch_major accordingly and
remove the duplicated loops.

source/usr/local/emhttp/plugins/nvidia-driver/include/card-support.php (1)

412-416: Variable shadowing in loop reduces readability.

Line 413 reassigns $version inside the foreach loop that iterates over $candidate_versions. While this works correctly, it shadows the loop variable and can confuse readers or cause subtle bugs if the loop body is extended later.

♻️ Proposed fix

   foreach ($candidate_versions as $version) {
-    $version = nvidia_extract_version($version);
-    if ($version === null) {
+    $normalized = nvidia_extract_version($version);
+    if ($normalized === null) {
       continue;
     }
 
-    $check = nvidia_driver_device_id_check_from_nvidia($version, $normalized_id, $fetch_timeout);
+    $check = nvidia_driver_device_id_check_from_nvidia($normalized, $normalized_id, $fetch_timeout);
     $checked_versions[] = $check;
 
     if (!empty($check['supported'])) {
-      $valid_versions[] = $version;
+      $valid_versions[] = $normalized;
     }
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/include/card-support.php`
around lines 412 - 416, The foreach loop over $candidate_versions reassigns the
loop variable $version after calling nvidia_extract_version, causing variable
shadowing; change the inner variable to a new name (e.g., $extractedVersion or
$parsed_version) so the loop variable remains untouched, update the null-check
to use that new variable and replace subsequent uses of the extracted value
accordingly (refer to the foreach over $candidate_versions and the call to
nvidia_extract_version).

source/usr/local/emhttp/plugins/nvidia-driver/include/gpu-support.php (2)

142-145: Remote URL fetches from GitHub raw may be unreliable.

Line 144 fetches architecture mapping from github.com/unraid/unraid-nvidia-driver/raw/master/.... GitHub raw URLs can be rate-limited or blocked in some network environments, and using master branch means any change to the repo immediately affects production systems.

Consider:

1. Using a versioned/tagged URL or CDN
2. Documenting that the remote fetch is optional (local fallback exists)
3. Ensuring the 5-second timeout (line 67) doesn't cause UX issues when GitHub is slow

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/include/gpu-support.php` around
lines 142 - 145, The call to nvidia_load_architecture_mapping currently uses an
unpinned GitHub raw URL which can be rate-limited or changed unexpectedly;
update the call to use a versioned or tagged URL (or configurable CDN) instead
of the master raw URL, make the remote fetch optional with clear documentation
that a local file is the fallback, and surface/configure the existing 5-second
timeout used by the fetch so it can be increased or handled gracefully to avoid
UX hangs; reference the nvidia_load_architecture_mapping invocation and the
fetch timeout setting so you add a configurable URL/tag constant, a flag or
docstring noting the local fallback, and timeout handling around the remote
fetch.

---

149-166: Duplicated version-fetching logic with nvidia-driver.page.

Lines 149-166 duplicate the version-fetching shell commands and candidate_versions assembly that also appear in nvidia-driver.page (lines 15-54). This creates maintenance burden and potential for drift between the two files.

Consider extracting shared version-fetching logic into a reusable include or function within card-support.php.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/include/gpu-support.php` around
lines 149 - 166, Extract the duplicated version-fetching logic (the shell_exec
calls that set $latest_v, $latest_prb_v, $latest_nfb_v, $latest_nos_v, the
file('/tmp/nvidia_driver') read into $eachlines/$available_versions, and the
assembly of $candidate_versions) into a single reusable function (e.g.,
get_nvidia_versions()) inside card-support.php, export or include that file
where needed, then replace the block in gpu-support.php with a call that returns
$available_versions and $candidate_versions; ensure the new function accepts no
args and preserves the same trimming/array_filter behavior and empty-string
checks so callers (gpu-support.php and nvidia-driver.page) get identical
outputs.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

source/usr/local/emhttp/plugins/nvidia-driver/include/gpu-support.php (1)

190-196: Minor inefficiency: lspci is always executed even when cache is used.

The shell_exec('lspci -nn') runs unconditionally on line 190, but then the result may be overwritten by cached data on lines 192-195. Consider checking for the cache first to avoid the unnecessary shell execution.

🔧 Proposed fix

-$lspci_query = shell_exec('lspci -nn 2>/dev/null');
+$lspci_query = null;
 if (is_file('/tmp/lspci_output.txt')) {
   $cached_lspci = `@file_get_contents`('/tmp/lspci_output.txt');
   if ($cached_lspci !== false && trim($cached_lspci) !== '') {
     $lspci_query = $cached_lspci;
   }
 }
+if ($lspci_query === null) {
+  $lspci_query = shell_exec('lspci -nn 2>/dev/null');
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/include/gpu-support.php` around
lines 190 - 196, Avoid running shell_exec('lspci -nn 2>/dev/null')
unconditionally: first check the cache file '/tmp/lspci_output.txt' with
is_file() and read it via file_get_contents(); if the cache contains non-empty
data assign it to $lspci_query, otherwise call shell_exec('lspci -nn
2>/dev/null') to populate $lspci_query (and optionally write the output to the
cache). Update the logic around $lspci_query, is_file('/tmp/lspci_output.txt')
and `@file_get_contents`('/tmp/lspci_output.txt') so the shell command is only
executed when no valid cache exists.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page (1)

160-209: ⚠️ Potential issue | 🟠 Major

Disable update action when no compatible driver is selectable.

When compatibility intersection is empty, all radios become disabled, but the update action remains clickable. That can still submit an empty/invalid selection from changeVersion() (Line 73). Disable the button when no enabled option exists.

🔧 Proposed fix

     var selected = document.querySelector('input[name="drv_version"]:checked');
     if (selected && selected.disabled) {
       var firstEnabled = document.querySelector('input[name="drv_version"]:not(:disabled)');
       if (firstEnabled) {
         firstEnabled.checked = true;
       }
     }
+
+    var updateButton = document.querySelector('input[type="button"][value="Update & Download"]');
+    if (updateButton) {
+      var hasEnabledOption = !!document.querySelector('input[name="drv_version"]:not(:disabled)');
+      updateButton.disabled = !hasEnabledOption;
+    }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page` around
lines 160 - 209, When the compatibility intersection is empty all driver radio
inputs are disabled but the update action (the button that calls
changeVersion()) remains enabled; query that update button element (the DOM
element used to trigger changeVersion()) and set its disabled state based on
whether any enabled 'input[name="drv_version"]' exists. Concretely: after you
finish disabling/enabling radios (and after selecting firstEnabled), compute
hasEnabled =
Boolean(document.querySelector('input[name="drv_version"]:not(:disabled)'));
then set updateButton.disabled = !hasEnabled (and ensure you obtain updateButton
via the same selector/reference used elsewhere to invoke changeVersion()). Also
ensure you update this disabled state in both the incompatibleAll branch and the
openOption/hideOpen handling so the button reflects the final radio
availability.

source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page (1)

14-19: Harden version metadata reads with safe defaults.

This block assumes every key exists in $version_data. A partial payload can cascade into warnings and break later array operations. Please default missing values defensively.

💡 Proposed hardening

-$version_data = nvidia_get_nvidia_versions();
-$latest_v = $version_data['latest_v'];
-$latest_prb_v = $version_data['latest_prb_v'];
-$latest_nfb_v = $version_data['latest_nfb_v'];
-$latest_nos_v = $version_data['latest_nos_v'];
+$version_data = nvidia_get_nvidia_versions();
+if (!is_array($version_data)) {
+  $version_data = [];
+}
+$latest_v = (string)($version_data['latest_v'] ?? '');
+$latest_prb_v = (string)($version_data['latest_prb_v'] ?? '');
+$latest_nfb_v = (string)($version_data['latest_nfb_v'] ?? '');
+$latest_nos_v = (string)($version_data['latest_nos_v'] ?? '');
@@
-$available_versions = $version_data['available_versions'];
-$candidate_versions = $version_data['candidate_versions'];
+$available_versions = is_array($version_data['available_versions'] ?? null) ? $version_data['available_versions'] : [];
+$candidate_versions = is_array($version_data['candidate_versions'] ?? null) ? $version_data['candidate_versions'] : [];

Also applies to: 34-37, 46-49

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page` around
lines 14 - 19, The assignments from nvidia_get_nvidia_versions() assume all keys
exist and can error on partial payloads; update the code that sets $latest_v,
$latest_prb_v, $latest_nfb_v, $latest_nos_v (and the analogous assignments later
in the file) to read each key defensively and provide a safe default when the
key is missing or null (e.g. use isset()/array_key_exists() or the
null-coalescing equivalent) so downstream array operations never receive
undefined values.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page (1)

268-269: Orphaned closing tags without matching openers.

These </tr> and </table> tags don't have corresponding opening tags in the current structure. The next <table> opens on line 293. This appears to be leftover from previous changes and should be removed for cleaner HTML structure.

♻️ Proposed fix

-        </tr>
-    </table>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page` around
lines 268 - 269, Remove the orphaned closing tags "</tr>" and "</table>" that
appear before the next "<table>" element; these stray closers have no matching
openers and should be deleted so the HTML structure is valid (search for the
standalone "</tr>" and "</table>" entries in the block and remove them), then
verify the surrounding markup still balances and validate the HTML.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page (1)

90-100: ⚠️ Potential issue | 🟠 Major

Disable the action when filtering leaves no valid radio.

If filtering disables every drv_version option, the previously checked radio can remain selected and Update & Download stays clickable. changeVersion() can still consume that stale value.

🔧 Proposed change

 function resetDriverOptionSupport() {
     var options = document.querySelectorAll('input[name="drv_version"]');
     options.forEach(function(option) {
       option.disabled = false;
       var parent = option.closest('p');
       if (parent) {
         parent.style.opacity = '1';
         parent.style.display = '';
       }
     });
+    var updateButton = document.querySelector('input[type="button"][value="Update & Download"]');
+    if (updateButton) {
+      updateButton.disabled = false;
+    }
 }
 ...
+    var updateButton = document.querySelector('input[type="button"][value="Update & Download"]');
     var selected = document.querySelector('input[name="drv_version"]:checked');
-    if (selected && selected.disabled) {
-      var firstEnabled = document.querySelector('input[name="drv_version"]:not(:disabled)');
-      if (firstEnabled) {
-        firstEnabled.checked = true;
-      }
-    }
+    var firstEnabled = document.querySelector('input[name="drv_version"]:not(:disabled)');
+    if (selected && selected.disabled) {
+      if (firstEnabled) {
+        firstEnabled.checked = true;
+      } else {
+        selected.checked = false;
+      }
+    }
+    if (updateButton) {
+      updateButton.disabled = !firstEnabled;
+    }

Also applies to: 211-259

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@source/usr/local/emhttp/plugins/nvidia-driver/nvidia-driver.page` around
lines 90 - 100, When filtering can disable every 'drv_version' option,
resetDriverOptionSupport must also clear any previously checked/stale radio and
prevent changeVersion() from reading it: inside resetDriverOptionSupport iterate
inputs[name="drv_version"] and if none remain enabled, remove the checked
attribute (and set checked=false) from any radio, and disable/hide the UI action
that triggers changeVersion (the Update & Download control) so it cannot be
clicked; also ensure changeVersion() only reads a radio that is both checked and
not disabled. Apply the same fix where filtering logic exists (the block around
functions referenced at lines 211-259).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes