Parametrize all monitors

ieure commented 4 years ago

@Zzull FYI, I've been substantially reworking Symon for a while, and it has some of the same things you were working on.

I reached out to @zk-phi, but didn't get any response, so I'm probably going to rename my fork and keep hacking on it instead of trying to contribute back. My work is at https://github.com/ieure/symon

pmiddend commented 3 years ago

@Zzull FYI, I've been substantially reworking Symon for a while, and it has some of the same things you were working on.

I reached out to @zk-phi, but didn't get any response, so I'm probably going to rename my fork and keep hacking on it instead of trying to contribute back. My work is at https://github.com/ieure/symon

Thank you very much for the rewrite. Can you enable the "Issues" tab in your fork? I tried using your fork via:

(use-package symon
  :straight (:host github :repo "ieure/symon")
  :config (symon-mode 1))

But got an error:

Debugger entered--Lisp error: (file-missing "Cannot open load file" "No such file or directory" "symon-sparkline")
  require(symon-sparkline)

ieure commented 3 years ago

@Zzull FYI, I've been substantially reworking Symon for a while, and it has some of the same things you were working on. I reached out to @zk-phi, but didn't get any response, so I'm probably going to rename my fork and keep hacking on it instead of trying to contribute back. My work is at https://github.com/ieure/symon

Thank you very much for the rewrite. Can you enable the "Issues" tab in your fork? I tried using your fork via:
(use-package symon
  :straight (:host github :repo "ieure/symon")
  :config (symon-mode 1))
But got an error:
Debugger entered--Lisp error: (file-missing "Cannot open load file" "No such file or directory" "symon-sparkline")
  require(symon-sparkline)

This should be fixed. I've moved my primary dev work of GitHub, so that's a read-only mirror of my private Gitea instance, so I'm not sure I want to enable issues.

pmiddend commented 3 years ago

@ieure Have you changed your code after I wrote to you? The latest commit in the repo is from September 23rd. So the issue doesn't seem to be fixed. I, however, figured out a workaround using an experimentally chosen require order:

(add-to-list 'load-path "~/code/symon")
(require 'symon-time)
(require 'symon-monitor)
(require 'symon-cpu)
(require 'symon-cpufreq)
(require 'symon-fan)
(require 'symon-memory)
(require 'symon-network)
(require 'symon-sparkline)
(require 'symon-temp)
(define-minor-mode symon-mode
  "Tiny graphical system monitor"
  :init-value nil
  :global t
  (if symon-mode (symon--initialize) (symon--cleanup)))
(require 'symon)
(require 'symon-battery)

Regarding GitHub/Gitea. I get that you moved away from GitHub, but I think it would be great if you made your work more public.

ieure commented 3 years ago

@ieure Have you changed your code after I wrote to you?

I haven't changed the code, but I pushed the changes I had already made (which I believe fix your problem) to the public repo.

The latest commit in the repo is from September 23rd.

That's that last time I committed anything, but the GitHub repo was out of date, so it was missing several months of commits.

So the issue doesn't seem to be fixed.

Have you tried again, or are you assuming that the commit date being before your report of issues means it's not fixed?

The version now on my GitHub is what I use every day. Not saying this means there are no problems, but I think the specific problem you hit is resolved.

Here's my config:

(use-package symon
  :straight (symon :repo "my-private-repo-but-put-github-here")

  :config
  (setq symon-monitors
        '(((symon-time :display-opts '(:format "%a %b %d %H:%M"))
           (symon-battery)

           (symon-cpufreq-linux)

           (when-let ((coretemp (symon-temp-coretemp-device)))
             (symon-temp :device coretemp :display-opts '(:index nil)))

           (when-let ((gputemp (symon-temp-find-name "nouveau")))
             (symon-temp :device gputemp :display-opts '(:index "GPU:")))

           (when-let ((fans (symon-fan-fans)))
             (symon-fan :fan (car fans) :display-opts '(:index nil :sparkline (:type gridded :upper-bound 6000.0))))

           (symon-cpu-linux :display-opts '(:sparkline (:type gridded)))
           (symon-memory-linux :display-opts '(:sparkline nil))
           (symon-emms))))
  (symon-mode))

Regarding GitHub/Gitea. I get that you moved away from GitHub, but I think it would be great if you made your work more public.

Open to suggestions if you have something concrete, but I won't be using GitHub as a primary platform for anything ever again.

Zzull commented 3 years ago

@ieure Have you considered Gitlab?

pmiddend commented 3 years ago

@ieure I have now tried using your exact code, but with the GitHub repository of course. To be completely sure, the commit hash of my version is 0d967a75c43dbfc47207e70fe329cce525a1b077. I get the following error on startup:

Error (use-package): symon/:catch: Cannot open load file: No such file or directory, symon-sparkline

ieure commented 3 years ago

@ieure I have now tried using your exact code, but with the GitHub repository of course. To be completely sure, the commit hash of my version is 0d967a75c43dbfc47207e70fe329cce525a1b077. I get the following error on startup:
Error (use-package): symon/:catch: Cannot open load file: No such file or directory, symon-sparkline

I poked at this today and I think it should be fixed.

Zzull commented 3 years ago

Hello @ieure,

I switched to your symon this weekend (commit 8a25d74). Thank you for the work you put in there. It also helped me learn some elisp along the way as you seem seasoned with it. I bumped into some small issues (mainly three) though that I'll sum up here.

As @pmiddend already mentioned, I also had to require multiple features for it to work. Simply requiring symon didn't do the job for some reason. Maybe it's just a matter of requiring all of them in symon.el. There was another missing feature for me in symon-cpu: cc-defs required by c-concat-separated.
I had a Symbol's value as variable is void: symon-mode that I pinned down to the following function:
```
(defun symon--set-and-restart (sym value)
(set-default sym value)
(when symon-mode
(symon-mode -1)
(symon-mode 1)))
```
Removing the when sexp allowed me to go further.
I finally had to prepend cl- to the functions shown below. I don't really get this, I have all of those functions accessible in my Emacs but named in a slightly different way. Did you require something in your own config to rename them? Is it just because we just don't share the same version of Emacs (mine's 27.1)?
```
diff --git a/symon-cpu.el b/symon-cpu.el
index d26bf38..91febf1 100644
--- a/symon-cpu.el
+++ b/symon-cpu.el
```

@@ -74,10 +75,10 @@ (mapcar #'symon-cpu--freq cpus))

(defun symon-cpu--minfreq (&optional cpus)

(/ (reduce #'min (mapcar #'car (symon-cpu--freqs (car (symon-cpu--cpus))))) 1000.0))
(/ (cl-reduce #'min (mapcar #'car (symon-cpu--freqs (car (symon-cpu--cpus))))) 1000.0))

(defun symon-cpu--maxfreq (&optional cpus)
(/ (reduce #'max (mapcar #'cadr (symon-cpu--freqs (car (symon-cpu--cpus))))) 1000.0))
(/ (cl-reduce #'max (mapcar #'cadr (symon-cpu--freqs (car (symon-cpu--cpus))))) 1000.0))

(defclass symon-cpu-linux (symon-monitor-history) ((last-total-ticks :type integer :initform 0) diff --git a/symon-cpufreq.el b/symon-cpufreq.el index 1c0e598..ee6a2da 100644 --- a/symon-cpufreq.el +++ b/symon-cpufreq.el @@ -39,10 +39,10 @@ (mapcar #'symon-cpu--freq cpus))

(defun symon-cpu--minfreq (&optional cpus)
(/ (reduce #'min (mapcar #'car (symon-cpu--freqs (car (symon-cpu--cpus))))) 1000.0))
(/ (cl-reduce #'min (mapcar #'car (symon-cpu--freqs (car (symon-cpu--cpus))))) 1000.0))

(defun symon-cpu--maxfreq (&optional cpus)
(/ (reduce #'max (mapcar #'cadr (symon-cpu--freqs (car (symon-cpu--cpus))))) 1000.0))
(/ (cl-reduce #'max (mapcar #'cadr (symon-cpu--freqs (car (symon-cpu--cpus))))) 1000.0))

;;;###autoload (defclass symon-cpufreq-linux (symon-monitor-history) @@ -63,7 +63,7 @@ (cl-defmethod symon-monitor-fetch ((this symon-cpufreq-linux)) "Return average of all cores in MHz." (with-slots (cpus) this
(/ (reduce #'+ (mapcar #'caddr (symon-cpu--freqs cpus))) (length cpus) 1000.0)))
(/ (cl-reduce #'+ (mapcar #'caddr (symon-cpu--freqs cpus))) (length cpus) 1000.0)))

(cl-defmethod symon-monitor-display ((this symon-cpufreq-linux)) (with-slots (cpus) this diff --git a/symon-monitor.el b/symon-monitor.el index d6e0501..dbe8db1 100644 --- a/symon-monitor.el +++ b/symon-monitor.el @@ -148,8 +148,8 @@ Subsequent errors of the same type are suppressed.")) (error (symon-monitor--maybe-warn this error 'fetch-errors-warned "Update")))))

(defun symon-monitor--plist-merge (defaults user)
(let ((opts (copy-list defaults))
(user (copy-list user)))
(let ((opts (cl-copy-list defaults))
(user (cl-copy-list user))) (while user (let ((k (pop user)) (v (pop user)))

ieure commented 3 years ago

Hello @ieure ...

Thanks for the patch, I applied your changes and pushed. Also symon--set-and-restart should be resilient to the issue you mentioned.

I'm still not sure about the requiring stuff. The relevant monitors are all marked autoload, so they should get pulled in on demand, but maybe those interact strangely with whatever defclass is expanding to under the hood.

If you want to stop polluting this issue, you're very welcome to email me patches/issues/feedback directly. My email is in code.

Zzull commented 3 years ago

If you want to stop polluting this issue …

I would actually like to close those two PRs now that they are superseded by your branch but I am afraid of the lack of discoverability it would entail for your version. How would people after that find out there actually is a working and maintained version of Symon?

Zzull commented 2 years ago

Hello @ieure,

Have you received an email I sent you about a month ago? Its subject is "[PATCH] Fix Symon network monitors". I sent it to the email found in the source

Zzull commented 2 years ago

Closing in favor of https://gitlab.com/ieure/lemon/

zk-phi / symon

Parametrize all monitors #39