andresgongora / synth-shell-greeter

A system status report and your personal ASCII-art logo for your terminal sessions
GNU General Public License v3.0
14 stars 15 forks source link

syntax error and not complete greeting #32

Open qrockz opened 3 years ago

qrockz commented 3 years ago

Fresh manual install and got this incomplete greeting. Screenshot from 2021-09-28 14-53-48

EDIT: When i remove CPUTEMP the script works fine. No errors then.

andresgongora commented 3 years ago

I'm on the go right now. I'll try to get back to you asap :) Meanwhile, could you check if you have lm-sensors (the name might be different on your distro) installed? I see that the temp is not shown. In any case, there should be no syntax error, that's odd

On Tue 28 Sep 2021, 14:55 qrockz, @.***> wrote:

Fresh manual install and got this incomplete greeting. [image: Screenshot from 2021-09-28 14-53-48] https://user-images.githubusercontent.com/56853194/135090811-b6032d53-efea-4cfc-be0b-8bb614469c81.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/andresgongora/synth-shell-greeter/issues/32, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC36INV5F4TF5P3RVSPIHITUEG3KNANCNFSM5E5LF5XA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

qrockz commented 3 years ago

Screenshot from 2021-09-29 07-11-32

andresgongora commented 3 years ago

Hmmm te quick solution is to disable te temperature monitor from the configuration. I have to take a good look at it, not sure what's going wrong. Maybe the syntax of lm-sensoes and sensors-detect is slightly different. Thanks for the screenshots :)

On Wed 29 Sep 2021, 07:12 qrockz, @.***> wrote:

[image: Screenshot from 2021-09-29 07-11-32] https://user-images.githubusercontent.com/56853194/135207108-f601e511-496c-4033-b6a6-bca1eae61bb9.png

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/andresgongora/synth-shell-greeter/issues/32#issuecomment-929841709, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC36INXRPX6SLMKUPO3ZM6TUEKN3NANCNFSM5E5LF5XA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

cybex-dev commented 2 years ago

Since this is a newer issue, I'll post additional info here.

Firstly, awesome extension, many thanks for creating it.

I have the same issue, looking at lm-sensors it seems it is expected for Ryzen CPUs, see https://bbs.archlinux.org/viewtopic.php?id=266997 for more info, thread from 2021/06.

Setup: AMD Ryzen 3600 (OC 4.2)

sensors output:

$ sensors
radeon-pci-0a00
Adapter: PCI adapter
temp1:        +39.0°C  (crit = +120.0°C, hyst = +90.0°C)

nvme-pci-0100
Adapter: PCI adapter
Composite:    +46.9°C  (low  = -273.1°C, high = +84.8°C)
                       (crit = +84.8°C)
Sensor 1:     +46.9°C  (low  = -273.1°C, high = +65261.8°C)
Sensor 2:     +52.9°C  (low  = -273.1°C, high = +65261.8°C)

iwlwifi_1-virtual-0
Adapter: Virtual device
temp1:        +44.0°C  

k10temp-pci-00c3
Adapter: PCI adapter
Tctl:         +46.1°C  
Tccd1:        +40.8°C 

The author suggests AMD CPUs will only report 1 temperature even though there may be multiple cores, the device is represented with the "k10temp-pci-00c3" in my case but digging around there is very little similar problems. Many of the out-of-box solutions work for even AMD systems, wondering if something isn't wrong with my setup.

Reviewing some askubuntu, reddit, linuxquestions threads, it seems there is some info for the k10temp sensor.

andresgongora commented 2 years ago

Let's have a look at it

        ## GET VALUES
        local temp_line=$(sensors 2>/dev/null |\
                          grep Core |\
                          head -n 1 |\
                          sed 's/^.*:[ \t]*//g;s/[\(\),]//g')
        local units=$(echo $temp_line |\
                      sed -n 's/.*\( [[CF]]*\).*/\1/p' |\
                      sed 's/\ /°/g')
        local current=$(echo $temp_line |\
                        sed -n 's/^.*+\(.*\) [[CF]]*[ \t]*h.*/\1/p')
        local high=$(echo $temp_line |\
                     sed -n 's/^.*high = +\(.*\) [[CF]]*[ \t]*c.*/\1/p')
        local max=$(echo $temp_line |\
                    sed -n 's/^.*crit = +\(.*\) [[CF]]*[ \t]*.*/\1/p')

This is the part of the script that gets the temp and stores it under current . It seems the problem is that the CPU temp is being identified only if it contains the word Core, as in my system:

Adapter: ISA adapter
Package id 0:  +32.0°C  (high = +85.0°C, crit = +105.0°C)
Core 0:        +31.0°C  (high = +85.0°C, crit = +105.0°C)
Core 1:        +24.0°C  (high = +85.0°C, crit = +105.0°C)
Core 2:        +30.0°C  (high = +85.0°C, crit = +105.0°C)
Core 3:        +29.0°C  (high = +85.0°C, crit = +105.0°C)

I'd like the script to be "automatic", but worst case, I could implement a different approach. Still, having the script detect keywords to identify the CPU is much simpler since sensors does not seems o be consistent over the board... unless we find out how we can ask sensors to use a more specific format.

Could you give me hand on this? I'll gladly implement it or merge your pull-request if you want to become a contributor