Skip to content

Comments

Dynamic smonctl#6

Closed
Kek5chen wants to merge 13 commits intomasterfrom
dynamic-smonctl
Closed

Dynamic smonctl#6
Kek5chen wants to merge 13 commits intomasterfrom
dynamic-smonctl

Conversation

@Kek5chen
Copy link
Contributor

@Kek5chen Kek5chen commented Apr 14, 2025

Implements #5

Squeezed this one in today. Tested in Lab. Should be fine but have a look.

@Kek5chen Kek5chen added the enhancement New feature or request label Apr 14, 2025
@Kek5chen Kek5chen requested a review from loulecrivain April 14, 2025 14:18
@Kek5chen Kek5chen self-assigned this Apr 14, 2025
@Kek5chen
Copy link
Contributor Author

README changes are missing.

Copy link
Contributor

@loulecrivain loulecrivain left a comment

Choose a reason for hiding this comment

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

hi, no problem with the code as far as I can see (I'm not very good with Go!). however I'd like us to be more feature-complete regarding smonctl output parsing (please see comments).

IMO it doesn't seem a lot more code to add PSU state support. I know in the past with hwmon.yml it was a raw type configured by hand, but we also need to parse this automatically.

thanks for your work!

@loulecrivain
Copy link
Contributor

README changes are missing.

Oh yes, and also, while we're at it, please do this too 🌞

@Kek5chen Kek5chen requested a review from loulecrivain April 23, 2025 13:31
Copy link
Contributor

@loulecrivain loulecrivain left a comment

Choose a reason for hiding this comment

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

This is not working on latest Cumulus Linux. Smonctl uses different output formats between versions (probably because of changes in other tool/daemons used by CL), and thus there's no driver_hwmon nor a driver_path in its output on the latest version. See for yourself:

  • on LSCL0001
      {
          "name": "PSU1", 
          "start_time": 1725878482, 
          "psu1_pwr_status": 1, 
          "state": "OK", 
          "prev_state": "OK", 
          "prev_msg": null, 
          "msg": null, 
          "psu1_power": 34, 
          "log_time": 1725878482, 
          "type": "power", 
          "psu1_status": 1, 
          "description": "PSU1"
      }
  • on LSCE0001:
        {
          "driver_hwmon": [
              "psu_pwr1"
          ], 
          "name": "PSU1", 
          "start_time": 1743596939, 
          "psu_pwr1_all_ok": "1", 
          "state": "OK", 
          "prev_state": "OK", 
          "psu_pwr1_present": "1", 
          "msg": null, 
          "prev_msg": null, 
          "log_time": 1743596939, 
          "driver_path": "/sys/bus/i2c/devices/0-0030", 
          "type": "power", 
          "description": "PSU1"
      }

IMO it'd be better to change the architecture here, and to rely on values which are directly on smonctl's output instead of parsing them ourselves from /sys 🤔 They have used the same keys for values over different versions.

@loulecrivain
Copy link
Contributor

other than that, I have no other comments regarding specific parts of the code.

@Kek5chen
Copy link
Contributor Author

rebased on master

@Kek5chen
Copy link
Contributor Author

I'm going to make a new PR so these changes land on master. It's a bit less cluttered

Kek5chen added a commit that referenced this pull request Apr 23, 2025
commit 9385ad0
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 17:20:21 2025 +0200

    hwmon: use wait group properly

commit 4abc9bf
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 17:07:01 2025 +0200

    Revert "README: update to v1.0.12"

    This reverts commit 9ff042d.

commit 25d7105
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 17:04:13 2025 +0200

    Revert "changelog: add entry"

    This reverts commit 5687a42.

commit db6a7f1
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 17:03:54 2025 +0200

    Revert "feat: dynamic smonctl"

    This reverts commit 44797cb.

commit f1af90a
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 15:40:13 2025 +0200

    README: fix note

commit c30d308
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 15:32:26 2025 +0200

    hwmon: name metrics properly

commit d102c8a
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 15:29:21 2025 +0200

    README: update to v1.0.12

commit bd89e15
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 15:28:04 2025 +0200

    README: remove asic collector warning

commit baa9110
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 15:21:43 2025 +0200

    changelog: add entry

commit 4f81f4f
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 15:20:08 2025 +0200

    hwmon: add power values

commit 451ced4
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Wed Apr 23 15:17:43 2025 +0200

    refactor: align assignments

commit ff07bdb
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Mon Apr 14 16:17:24 2025 +0200

    feat: dynamic smonctl

commit a43a0c5
Author: willow <52585984+Kek5chen@users.noreply.github.com>
Date:   Mon Apr 14 16:16:47 2025 +0200

    refactor: cleanup hwmon
@Kek5chen
Copy link
Contributor Author

manually merged

@Kek5chen Kek5chen closed this Apr 23, 2025
@Kek5chen Kek5chen deleted the dynamic-smonctl branch April 23, 2025 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants