Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Conky for mac os master #579

Merged
merged 18 commits into from
Aug 7, 2018
Merged

Conky for mac os master #579

merged 18 commits into from
Aug 7, 2018

Conversation

brndnmtthws
Copy link
Owner

No description provided.

@brndnmtthws brndnmtthws mentioned this pull request Aug 7, 2018
Closed
brndnmtthws
brndnmtthws commented Aug 7, 2018
View reviewed changes
Copy link
Owner Author

@brndnmtthws brndnmtthws left a comment

Choose a reason for hiding this comment

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

@npyl one minor code nit pick. Otherwise, everything looks great.

src/core.cc Outdated
@@ -121,6 +121,12 @@
#include "darwin.h"
#endif

#if defined (__APPLE__) && defined (__MACH__)
#define DARWIN_DEALLOCATE_CPU_SAMPLE obj->callbacks.free = &deallocate_cpu_sample;
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is a nit pick, but can you change this to a function instead of a macro? We have too many crazy C macros.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please give me an example because I am not sure how this would work? If I understand correct, you want me to introduce a general function e.g. deallocate_cpu but this would mean that all OS-specific files (linux.cc, freebsd.cc, etc) should introduce this function, too!

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes precisely, an OS-dependent function (like deallocate_cpu) which can be set as the free callback, like this:

conky/src/core.cc

Lines 579 to 582 in 4d5db32

END OBJ_ARG(disk_protect, 0, "disk_protect needs an argument") obj->data.s =
strndup(dev_name(arg), text_buffer_size.get(*state));
obj->callbacks.print = &print_disk_protect_queue;
obj->callbacks.free = &gen_free_opaque;

Copy link
Owner Author

Choose a reason for hiding this comment

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

This function can just be a no-op on other platforms btw.

This is mostly about establishing better practices/patterns. I try to make sure everything continually improves :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok! I am on it now!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@brndnmtthws I got it! 😉

@brndnmtthws brndnmtthws force-pushed the Conky-for-macOS-master branch from d7553dc to 1150ddf Compare August 7, 2018 14:22
brndnmtthws
brndnmtthws commented Aug 7, 2018
View reviewed changes
src/darwin.mm Outdated
@@ -286,7 +286,7 @@ void allocate_cpu_sample(struct cpusample **sample) {
sample_handle = *sample; /* use a public handle for deallocating */
}

void deallocate_cpu_sample(struct text_object *obj) {
void free_cpu(struct text_object *) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Brilliant, thanks!

Repository owner deleted a comment from brndnmtthws Aug 7, 2018
Repository owner deleted a comment from npyl Aug 7, 2018
npyl and others added 18 commits August 7, 2018 11:45
@npyl @brndnmtthws
Try to amend #31
f688edd
@npyl @brndnmtthws
BUILD_WLAN should be available for all OS.
306ded2 
Keep BUILD_WLAN OFF by default for compatibility reasons.
@npyl @brndnmtthws
WLAN-related variables should be available for every OS.
c2b4cbd 
There are some problems (probably null-dereference)
@npyl @brndnmtthws
Fix $wireless_essid crashing conky if no argument provided.
a1d0793 
Conky wasn't parsing the argument of the variable as it should, thus wasn't allocating the `dev` member variable.

Also fix some documentation stuff.
@npyl @brndnmtthws
Improve get_freq #20
6e5dbf6 
Using the Intel® Power Gadget API (https://software.intel.com/en-us/blogs/2012/12/13/using-the-intel-power-gadget-api-on-mac-os-x) we can now get actual Core frequency and not the constant factory one.

Though, for some weird reason the API gives the same freq for all Cores, thus the |cpu| arg becomes useless.
@npyl @brndnmtthws
Oops, this accidently slipped in
70f530d
@npyl @brndnmtthws
Introduce BUILD_IPGFREQ build option
be8d41d 
This build option has been introduced for one particular reason:

On macOS getting current core-frequency is not supported by the APIs.  A solution is to install Intel's ® Power Gadget which comes with an .app, a Framework and a kernel-extension.  Though, this may trouble some alot, thus introduce BUILD_IPGFREQ.
@npyl @brndnmtthws
Forgot static here.
b267fa4
@npyl @brndnmtthws
Some improvements for get_freq again.
df52a17 
Fix frequency not printing correctly (I wasn't using the divisor)
Add more guards.
@npyl @brndnmtthws
Setup cmake files and project code for Objective-C code #17
8f2272c 
We want to use CoreWLAN framework.
@npyl @brndnmtthws
update_cpu_usage() now supports multiple cores
2942e67 
Also, some cleanup.
@TheFatDemon @brndnmtthws
Updated default conky config to monitor Mac Networking
6adf22f
@TheFatDemon @brndnmtthws
Made Mac Friendly BuildOptions and generic default conky configs
ef1ad1b
@TheFatDemon @brndnmtthws
Undid Xdamage config and cleaned up previous IF statements
cc37fb9
@TheFatDemon @brndnmtthws
Re-Added XDamage fix
c546c84
@npyl @brndnmtthws
Finish up the algorithm. I think its now correct. Closes: #33
ce06769
@npyl @brndnmtthws
Cleanup macro and introduce a no-op free_cpu() function for ALL cpu-r…
03abfc6 
…elated variables

free_cpu() must be implemented for every OS and on all except macOS its a no-op function.
@brndnmtthws
Reformat, add empty comment.
d9b218b
@brndnmtthws brndnmtthws force-pushed the Conky-for-macOS-master branch from b417f59 to d9b218b Compare August 7, 2018 15:47
@brndnmtthws
Copy link
Owner Author

Sorry, I made a mess of the PR. I think we are good to go now though.

@brndnmtthws brndnmtthws merged commit 8aa9c81 into master Aug 7, 2018
@brndnmtthws brndnmtthws deleted the Conky-for-macOS-master branch August 7, 2018 15:54
lasers pushed a commit that referenced this pull request Aug 12, 2018
@brndnmtthws @lasers
Conky for mac os master (#579)
dce0ffb 
* Try to amend #31

* BUILD_WLAN should be available for all OS.

Keep BUILD_WLAN OFF by default for compatibility reasons.

* WLAN-related variables should be available for every OS.

There are some problems (probably null-dereference)

* Fix $wireless_essid crashing conky if no argument provided.

Conky wasn't parsing the argument of the variable as it should, thus wasn't allocating the `dev` member variable.

Also fix some documentation stuff.

* Improve `get_freq` #20

Using the Intel® Power Gadget API (https://software.intel.com/en-us/blogs/2012/12/13/using-the-intel-power-gadget-api-on-mac-os-x) we can now get actual Core frequency and not the constant factory one.

Though, for some weird reason the API gives the same freq for all Cores, thus the |cpu| arg becomes useless.

* Oops, this accidently slipped in

* Introduce BUILD_IPGFREQ build option

This build option has been introduced for one particular reason:

On macOS getting current core-frequency is not supported by the APIs.  A solution is to install Intel's ® Power Gadget which comes with an .app, a Framework and a kernel-extension.  Though, this may trouble some alot, thus introduce BUILD_IPGFREQ.

* Forgot static here.

* Some improvements for get_freq again.

Fix frequency not printing correctly (I wasn't using the divisor)
Add more guards.

* Setup cmake files and project code for Objective-C code #17

We want to use CoreWLAN framework.

* update_cpu_usage() now supports multiple cores

Also, some cleanup.

* Updated default conky config to monitor Mac Networking

* Made Mac Friendly BuildOptions and generic default conky configs

* Undid Xdamage config and cleaned up previous IF statements

* Re-Added XDamage fix

* Finish up the algorithm. I think its now correct. Closes: #33

* Cleanup macro and introduce a no-op free_cpu() function for ALL cpu-related variables

free_cpu() must be implemented for every OS and on all except macOS its a no-op function.

* Reformat, add empty comment.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@npyl npyl npyl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@brndnmtthws @npyl @TheFatDemon