lptstr / winfetch

🛠 A command-line system information utility written in PowerShell. Like Neofetch, but for Windows.
MIT License
1.35k stars 76 forks source link

`get_percent_bar $percentage`: Cannot validate argument on parameter 'percent' #181

Open aetonsi opened 1 year ago

aetonsi commented 1 year ago

Hi, i'm having the error mentioned when invoking winfetch the first time after each boot. Afterwards, it works just fine.

I have created a small function to invoke winfetch with my preferred parameters:

function Show-Winfetch([switch] $Short) {
  if ($Short) {
    $params = @{
      showpkgs = ''
    }
  } else {
    $params = @{
      all       = $true
      showdisks = @('*')
      showpkgs  = @('scoop', 'choco')
    }
  }
  & winfetch @params -blink -memorystyle textbar -cpustyle textbar -diskstyle textbar -batterystyle textbar
}

Then i simply call Show-Winfetch in one of my startup scripts. It prints this:

 lllllllllllllll   lllllllllllllll  user@COMPUTERNAME
 lllllllllllllll   lllllllllllllll  -------------
 lllllllllllllll   lllllllllllllll  OS: Windows 11 Pro N
 lllllllllllllll   lllllllllllllll  Host: HP HP Pavilion
 lllllllllllllll   lllllllllllllll  Kernel: 10.0.22621.0
 lllllllllllllll   lllllllllllllll  Motherboard: HP 87C5
 lllllllllllllll   lllllllllllllll  Uptime: 1 minute
                                    Resolution: 1536x864
 lllllllllllllll   lllllllllllllll  PS Packages: 4 Module
 lllllllllllllll   lllllllllllllll  Packages: 105 (choco)
 lllllllllllllll   lllllllllllllll  Shell: PowerShell v7.
 lllllllllllllll   lllllllllllllll  Terminal: Windows Ter
 lllllllllllllll   lllllllllllllll  Theme: Custom (System
 lllllllllllllll   lllllllllllllll  CPU: AMD Ryzen 5 4500
 lllllllllllllll   lllllllllllllll  GPU: AMD Radeon(TM) G
get_percent_bar : Cannot validate argument on parameter 'percent'. The 119 argument is greater than the maximum allowed range of 100. Supply an argument that is less than or equal to 100 and then try the command again.
At C:\tools\winfetch\winfetch.ps1:327 char:53
+         'textbar' { return "$text $(get_percent_bar $percentage)" }
+                                                     ~~~~~~~~~~~
+ CategoryInfo          : InvalidData: (:) [get_percent_bar], ParameterBindingValidationException
+ FullyQualifiedErrorId : ParameterArgumentValidationError,get_percent_bar
                                    CPU Usage: 299 proces
                                    Memory: 7.67 GiB / 19
                                    Disk (C:): 309 GiB /
                                    Disk (K:): 3 GiB / 15
                                    Disk (L:): 317 GiB /
                                    Battery: Discharging,
                                    Locale: Italy - Engli
                                    Weather: 6°C - Overca
                                    Local IP: 192.168.178
                                    Public IP: 99.17.56.1

The output is truncated like that because i am running the script inside a Windows Terminal split pane ($Host.UI.RawUI.WindowSize.Width = 58, $Host.UI.RawUI.WindowSize.Height = 30)


thank you for your time

rashil2000 commented 1 year ago

Can you try isolating which segment is causing the error? I think it's CPU usage, but please confirm.

aetonsi commented 1 year ago

how do i do that, exactly? i'll have to reboot the pc for each attempt so i have to be sure to make the correct edits...

rashil2000 commented 1 year ago

You can edit the config file located at ~/.config/winfetch/config.ps1

And you don't need to reboot every time - you can just run the script directly after editing the config file.

aetonsi commented 1 year ago

Hi, i've not been able to reproduce the issue again, i sincerely don't know what caused it, sorry

aetonsi commented 1 year ago

@rashil2000 it happened again but i have no idea why, i cannot reproduce it regularly😞 ... it's always after the GPU line and when running in a small window, though:

 lllllllllllllll   lllllllllllllll  user@computer
 lllllllllllllll   lllllllllllllll  -------------
 lllllllllllllll   lllllllllllllll  OS: Windows 11 Pro N
 lllllllllllllll   lllllllllllllll  Host: HP HP Pavilion
 lllllllllllllll   lllllllllllllll  Kernel: 10.0.22621.0
 lllllllllllllll   lllllllllllllll  Motherboard: HP 87C5
 lllllllllllllll   lllllllllllllll  Uptime: 2 minutes
                                    Resolution: 1536x864
 lllllllllllllll   lllllllllllllll  PS Packages: 5 Module
 lllllllllllllll   lllllllllllllll  Packages: 110 (choco)
 lllllllllllllll   lllllllllllllll  Shell: PowerShell v7.
 lllllllllllllll   lllllllllllllll  Terminal: Windows Ter
 lllllllllllllll   lllllllllllllll  Theme: Custom (System
 lllllllllllllll   lllllllllllllll  CPU: AMD Ryzen 5 4500
 lllllllllllllll   lllllllllllllll  GPU: AMD Radeon(TM) G
get_percent_bar : Cannot validate argument on parameter 'percent'. The 114 argument is greater than the maximum allowed range of 100. Supply an argument that is less than or equal to 100 and then try the command again.
At C:\tools\winfetch\winfetch.ps1:327 char:53
+         'textbar' { return "$text $(get_percent_bar $percentage)" }
+                                                     ~~~~~~~~~~~
+ CategoryInfo          : InvalidData: (:) [get_percent_bar], ParameterBindingValidationException
+ FullyQualifiedErrorId : ParameterArgumentValidationError,get_percent_bar
                                    CPU Usage: 316 proces
                                    Memory: 9.21 GiB / 19
                                    Disk (C:): 312 GiB /
                                    Disk (K:): 0 GiB / 15
                                    Disk (L:): 320 GiB /
                                    Battery: Discharging,
                                    Locale: Italy - Engli
                                    Weather: 9°C - Clear
                                    Local IP: 192.168.xxx
                                    Public IP: 2.36.xxx.x

what can i do to debug?

rashil2000 commented 1 year ago

You can edit the config file located at ~/.config/winfetch/config.ps1

Make a backup of this file somewhere. Then edit it to include just the following text:

@(
    cpu_usage
)

and now try to repro the issue

aetonsi commented 1 year ago

Hi, i think you meant "cpu_usage" (with quotes). This is winfetch output:

>winfetch

 lllllllllllllll   lllllllllllllll  CPU Usage: 18% (340 processes)
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll

 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll
 lllllllllllllll   lllllllllllllll

So, no error.... it doesn't always pop up anyways, only a few times, sporadically

rashil2000 commented 1 year ago

@Carterpersall any idea why the total comes up more than 100 here sometimes?

Carterpersall commented 1 year ago

@Carterpersall any idea why the total comes up more than 100 here sometimes?

Sorry about the delay, I don’t have a lot of time in the summer to code. I’m not really sure why it is doing that, but I do have two theories:

  1. The current implementation treats parent and child processes the same, which might lead to double counting the CPU usage of some programs. This does depend on parent processes also including the CPU time of the child processes, which may or may not be the case.
  2. The method might occasionally return garbage for some reason. I don’t like the idea of blaming this issue on Microsoft, but it is possible. It could be caused by the counter they use (if they use one) not having a high enough delay between the first and second read, which would sometimes create a garbage reading. It would also explain why it can happen on the first usage of it but not subsequent ones. I would need to look into the source of the System.Diagnostics.Process class (if available) to figure out if that could be the case.

As for a solution, I can see two possible avenues for now:

  1. Check if the CPU usage is over 100%, and if it is, set it to 100% to avoid the error.
  2. If CPU usage is over 100%, try executing the function again. If it fails again, either fail gracefully (print an error message) or set it to 100%.
Carterpersall commented 1 year ago

Here’s what I’ve found after looking into how [System.Diagnostics.Process]::GetProcesses() works:

It's possible that the source of the bug is that we are using each process's start time on a CPU time that is measured per-thread, which have their own individual start times. I threw together an implementation of info_cpu_usage that uses every thread instead of each process, but it seems to be about 2.5x slower. It also returns different results, but it may be more accurate due to the "more correct" usage of the CPU times. More testing is needed to confirm.

aetonsi commented 1 year ago

Well if 2.5x means a few milliseconds more, i'd say that's acceptable? i did measure what the current function took to complete and it was less than 15ms, so 2.5x would still be negligible IMHO