Nvidia updates, docs and fixes (#278)

* nvidia: Update the docs and add more arguments

**Updated the docs in variables.xml to reflect the current nvidia object.**

Basically just copied the inline comments into the XML document

**Added the last of the compatible arguments**

The following arguments were added to `get_nvidia_barval`:

* memfree
* ambienttemp
* gpufreq
* memfreq
* mtrfreq
* imagequality

This should leave only incompatible arguments unsupported (e.g. min/max
values).

Decided to base percentages off of zero rather than the specified minimum
values.

Signed-off-by: Matt Sturgeon <matt@sturgeon.me.uk>

* nvidia: Better error/debug printing

* Merged `scan_nvidia_args` and `set_nvidia_type` into new
  `set_nvidia_query`
* Added a translation array for `nvidia`, `nvidiabar`, `nvidiagauge` and
  `nvidiagraph` commands (helps with error printing)
* Added some NORM_ERR prints to `get_nvidia_barval`, `get_nvidia_value` and
  `get_nvidia_string`
* Removed exess newlines in nvidia's core.cc error definitions

Signed-off-by: Matt Sturgeon <matt@sturgeon.me.uk>

* nvidia: Better sanity checks when running queries

Introduced a pair of functions that checks whether the target we are
querying exists and how many of the target there are.

If the target doesn't exist there probably isn't a nvidia card
installed, if the target count query fails then the propietary drivers
probably aren't installed.

Fixes #269

Signed-off-by: Matt Sturgeon <matt@sturgeon.me.uk>

doc/variables.xml

@@ -2777,49 +2777,251 @@
             <command>
                 <option>nvidia</option>
             </command>
-            <option>threshold</option>
+            <option>argument</option>
-            <option>temp</option>
-            <option>ambient</option>
-            <option>gpufreq</option>
-            <option>memfreq</option>
-            <option>imagequality</option>
         </term>
-        <listitem>Nvidia graficcard support for the XNVCtrl
+        <listitem>Nvidia graphics card information via the XNVCtrl
-        library. Each option can be shortened to the least
+        library.
-        significant part. Temperatures are printed as float, all
+        <para />
-        other values as integer. 
+        <emphasis>Possible arguments:</emphasis> (Temperatures are
-        <simplelist>
+        printed as float, all other values as integer. Bracketed
+        arguments are aliases)
+        <simplelist type='horiz' columns='2'>
+            <!-- Temperatures -->
             <member>
-		<command>threshold</command>
+                <command>gputemp</command>
-		<option>The thresholdtemperature at
+                (<command>temp</command>)
-			which the gpu slows down</option>
+                <option>GPU temperature</option>
             </member>
             <member>
-		<command>temp</command>
+                <command>gputempthreshold</command>
-		<option>Gives the gpu current
+                (<command>threshold</command>)
-			temperature</option>
+                <option>Temperature threshold where the GPU will reduce it's clock speed</option>
             </member>
             <member>
-                <command>ambient</command>
+                <command>ambienttemp</command>
-                <option>Gives current air temperature near GPU
+                (<command>ambient</command>)
-                case</option>
+                <option>Ambient temperature outside the graphics card</option>
+            </member>
+
+            <!-- GPU frequency -->
+            <member>
+                <command>gpufreqcur</command>
+                (<command>gpufreq</command>)
+                <option>Current GPU clock speed</option>
             </member>
             <member>
-                <command>gpufreq</command>
+                <command>gpufreqmin</command>
-                <option>Gives the current gpu frequency</option>
+                <option>Minimum GPU clock speed</option>
             </member>
             <member>
-                <command>memfreq</command>
+                <command>gpufreqmax</command>
-                <option>Gives the current mem frequency</option>
+                <option>Maximum GPU clock speed</option>
             </member>
+
+            <!-- Memory frequency -->
+            <member>
+                <command>memfreqcur</command>
+                (<command>memfreq</command>)
+                <option>Current memory clock speed</option>
+            </member>
+            <member>
+                <command>memfreqmin</command>
+                <option>Minimum memory clock speed</option>
+            </member>
+            <member>
+                <command>memfreqmax</command>
+                <option>Maximum memory clock speed</option>
+            </member>
+
+            <!-- Memory transfer rate frequency -->
+            <member>
+                <command>mtrfreqcur</command>
+                (<command>mtrfreq</command>)
+                <option>Current memory transfer rate clock speed</option>
+            </member>
+            <member>
+                <command>mtrfreqmin</command>
+                <option>Minimum memory transfer rate clock speed</option>
+            </member>
+            <member>
+                <command>mtrfreqmax</command>
+                <option>Maximum memory transfer rate clock speed</option>
+            </member>
+
+            <!-- Performance levels -->
+            <member>
+                <command>perflevelcur</command>
+                (<command>perflevel</command>)
+                <option>Current performance level</option>
+            </member>
+            <member>
+                <command>perflevelmin</command>
+                <option>Lowest performance level</option>
+            </member>
+            <member>
+                <command>perflevelmax</command>
+                <option>Highest performance level</option>
+            </member>
+            <member>
+                <command>perfmode</command>
+                <option>Performance mode</option>
+            </member>
+
+            <!-- Load/utilization -->
+            <member>
+                <command>gpuutil</command>
+                <option>GPU utilization %</option>
+            </member>
+            <member>
+                <command>membwutil</command>
+                <option>Memory bandwidth utilization %</option>
+            </member>
+            <member>
+                <command>videoutil</command>
+                <option>Video engine utilization %</option><!-- ??? -->
+            </member>
+            <member>
+                <command>pcieutil</command>
+                <option>PCIe bandwidth utilization %</option>
+            </member>
+
+            <!-- RAM statistics -->
+            <member>
+                <command>memused</command>
+                (<command>mem</command>)
+                <option>Amount of used memory</option>
+            </member>
+            <member>
+                <command>memfree</command>
+                (<command>memavail</command>)
+                <option>Amount of free memory</option>
+            </member>
+            <member>
+                <command>memmax</command>
+                (<command>memtotal</command>)
+                <option>Total amount of memory</option>
+            </member>
+            <member>
+                <command>memutil</command>
+                (<command>memperc</command>)
+                <option>Memory utilization %</option>
+            </member>
+
+            <!-- Fan/cooler -->
+            <member>
+                <command>fanspeed</command>
+                <option>Fan speed</option>
+            </member>
+            <member>
+                <command>fanlevel</command>
+                <option>Fan level %</option>
+            </member>
+
+            <!-- Miscellaneous -->
             <member>
                 <command>imagequality</command>
-                <option>Which imagequality should be chosen by
+                <option>Image quality</option>
-                OpenGL applications</option>
             </member>
         </simplelist>
         <para /></listitem>
     </varlistentry>
+    <varlistentry>
+        <term>
+            <command>
+                <option>nvidiabar</option>
+            </command>
+            <option>(height),(width)</option>
+            <option>argument</option>
+        </term>
+        <listitem>Same as nvidia, except it draws its output in a
+        horizontal bar. The height and width parameters are optional,
+        and default to the default_bar_height and default_bar_width
+        config settings, respectively.
+        <para />
+        <emphasis>Note the following arguments are incompatible:</emphasis>
+        <simplelist type='horiz' columns='3'>
+            <member>
+                <command>gputempthreshold</command>
+                (<command>threshold</command>)
+            </member>
+            <member>
+                <command>gpufreqmin</command>
+            </member>
+            <member>
+                <command>gpufreqmax</command>
+            </member>
+            <member>
+                <command>memfreqmin</command>
+            </member>
+            <member>
+                <command>memfreqmax</command>
+            </member>
+            <member>
+                <command>mtrfreqmin</command>
+            </member>
+            <member>
+                <command>mtrfreqmax</command>
+            </member>
+            <member>
+                <command>perflevelmin</command>
+            </member>
+            <member>
+                <command>perflevelmax</command>
+            </member>
+            <member>
+                <command>perfmode</command>
+            </member>
+            <member>
+                <command>memtotal</command>
+                (<command>memmax</command>)
+            </member>
+            <member>
+                <command>fanspeed</command>
+            </member>
+        </simplelist>
+        <para /></listitem>
+    </varlistentry>
+    <varlistentry>
+        <term>
+            <command>
+                <option>nvidiagauge</option>
+            </command>
+            <option>(height),(width)</option>
+            <option>argument</option>
+        </term>
+        <listitem>Same as nvidiabar, except  a round gauge
+        (much like a vehicle speedometer). The height
+        and width parameters are optional, and default to the
+        default_gauge_height and default_gauge_width config
+        settings, respectively.
+        <para />
+        For possible arguments see nvidia and nvidiabar.
+        <para /></listitem>
+    </varlistentry>
+    <varlistentry>
+        <term>
+            <command>
+                <option>nvidiagraph</option>
+            </command>
+            <option>argument</option>
+            <option>(height),(width)</option>
+            <option>(gradient color 1)</option>
+            <option>(gradient color 2)</option>
+            <option>(scale)</option>
+            <option>(-t)</option>
+            <option>(-l)</option>
+        </term>
+        <listitem>Same as nvidiabar, except a horizontally
+        scrolling graph with values from 0-100 plotted on the
+        vertical axis. The height and width parameters are
+        optional, and default to the default_graph_height and
+        default_graph_width config settings, respectively.
+        <para />
+        For possible arguments see nvidia and nvidiabar. To learn more
+        about the -t -l and gradient color options,
+        see execgraph.
+        <para /></listitem>
+    </varlistentry>
     <varlistentry>
         <term>
             <command>

src/core.cc

@@ -1802,30 +1802,30 @@ struct text_object *construct_text_object(char *s, const char *arg,
 		obj->callbacks.free = &free_combine;
 #ifdef BUILD_NVIDIA
 	END OBJ_ARG(nvidia, 0, "nvidia needs an argument")
-		if (set_nvidia_type(obj, arg)) {
+		if (set_nvidia_query(obj, arg, NONSPECIAL)) {
 			CRIT_ERR(obj, free_at_crash, "nvidia: invalid argument"
-				 " specified: '%s'\n", arg);
+				 " specified: '%s'", arg);
 		}
 		obj->callbacks.print = &print_nvidia_value;
 		obj->callbacks.free = &free_nvidia;
 	END OBJ_ARG(nvidiabar, 0, "nvidiabar needs an argument")
-		if (scan_nvidia_args(obj, arg, BAR)) {
+		if (set_nvidia_query(obj, arg, BAR)) {
 			CRIT_ERR(obj, free_at_crash, "nvidiabar: invalid argument"
-				 " specified: '%s'\n", arg);
+				 " specified: '%s'", arg);
 		}
 		obj->callbacks.barval = &get_nvidia_barval;
 		obj->callbacks.free = &free_nvidia;
 	END OBJ_ARG(nvidiagraph, 0, "nvidiagraph needs an argument")
-		if (scan_nvidia_args(obj, arg, GRAPH)) {
+		if (set_nvidia_query(obj, arg, GRAPH)) {
 			CRIT_ERR(obj, free_at_crash, "nvidiagraph: invalid argument"
-				 " specified: '%s'\n", arg);
+				 " specified: '%s'", arg);
 		}
 		obj->callbacks.graphval = &get_nvidia_barval;
 		obj->callbacks.free = &free_nvidia;
 	END OBJ_ARG(nvidiagauge, 0, "nvidiagauge needs an argument")
-		if (scan_nvidia_args(obj, arg, GAUGE)) {
+		if (set_nvidia_query(obj, arg, GAUGE)) {
 			CRIT_ERR(obj, free_at_crash, "nvidiagauge: invalid argument"
-				 " specified: '%s'\n", arg);
+				 " specified: '%s'", arg);
 		}
 		obj->callbacks.gaugeval = &get_nvidia_barval;
 		obj->callbacks.free = &free_nvidia;

src/nvidia.cc

@@ -43,7 +43,6 @@
  *   so that all quirks are located there
  * - Implement nvs->print_type to allow control over how the value is printed
  *   (int, float, temperature...)
- * - Rename set_nvidia_type() to set_nvidia_query (requires changes in core.cc)
  * 
  * Showcase (conky.conf):
  * --==| NVIDIA | ==--
@@ -282,9 +281,31 @@ typedef enum _SEARCH_ID {
 	SEARCH_MAX
 } SEARCH_ID;
 
+// Translate special_type into command string
+const char* translate_nvidia_special_type[] = {
+ 	"nvidia", // NONSPECIAL
+ 	"", // HORIZONTAL_LINE
+ 	"", // STIPPLED_HR
+ 	"nvidiabar", // BAR
+ 	"", // FG
+ 	"", // BG
+ 	"", // OUTLINE
+ 	"", // ALIGNR
+ 	"", // ALIGNC
+ 	"nvidiagague", // GAUGE
+ 	"nvidiagraph", // GRAPH
+ 	"", // OFFSET
+ 	"", // VOFFSET
+ 	"", // FONT
+ 	"", // GOTO
+ 	"" // TAB
+};
+
 
 // Global struct to keep track of queries
 struct nvidia_s {
+	const char *command;
+	const char *arg;
 	QUERY_ID query;
 	TARGET_ID target;
 	ATTR_ID attribute;
@@ -340,11 +361,20 @@ namespace {
 	nvidia_display_setting nvidia_display;
 }
 
-// Extract arguments for nvidiabar, etc, and run set_nvidia_type
-int scan_nvidia_args (struct text_object *obj, const char *args, unsigned int special_t) {
-	const char *arg = args;
 
-	switch (special_t) {
+// Evaluate module parameters and prepare query
+int set_nvidia_query(struct text_object *obj, const char *arg, unsigned int special_type)
+{
+	struct nvidia_s *nvs;
+	int aid;
+
+	// Initialize global struct
+	obj->data.opaque = malloc(sizeof(struct nvidia_s));
+	nvs = static_cast<nvidia_s *>(obj->data.opaque);
+	memset(nvs, 0, sizeof(struct nvidia_s));
+
+	// Extract arguments for nvidiabar, etc, and run set_nvidia_query
+	switch (special_type) {
 		case BAR:
 			arg = scan_bar(obj, arg, 100);
 			break;
@@ -354,28 +384,11 @@ int scan_nvidia_args (struct text_object *obj, const char *args, unsigned int sp
 		case GAUGE:
 			arg = scan_gauge(obj, arg, 100);
 			break;
-		default:
-			return 1;
 	}
 
 	// Return error if no argument
 	// (sometimes scan_graph gets excited and eats the whole string!
 	if (!arg) return 1;
-
-	return set_nvidia_type(obj, arg);
-}
-
-
-// Evaluate module parameters and prepare query
-int set_nvidia_type(struct text_object *obj, const char *arg)
-{
-	struct nvidia_s *nvs;
-	int aid;
-
-	// Initialize global struct
-	obj->data.opaque = malloc(sizeof(struct nvidia_s));
-	nvs = static_cast<nvidia_s *>(obj->data.opaque);
-	memset(nvs, 0, sizeof(struct nvidia_s));
 	
 	// Translate parameter to id
 	for (aid=0; aid < ARG_UNKNOWN; aid++) {
@@ -384,6 +397,10 @@ int set_nvidia_type(struct text_object *obj, const char *arg)
 	}
 	//fprintf(stderr, "parameter: %s -> aid: %d\n", arg, aid);
 	
+	// Save pointers to the arg and command strings for dubugging and printing
+	nvs->arg = translate_module_argument[aid];
+	nvs->command = translate_nvidia_special_type[special_type];
+
 	// Evaluate parameter
 	switch(aid) {
 		
@@ -569,11 +586,37 @@ int set_nvidia_type(struct text_object *obj, const char *arg)
 			break;
 			
 		default:						// Unknown/invalid argument
+			// Error printed by core.cc
 			return 1;
 	}
 	return 0;
 }
 
+// Return the amount of targets present (or -1 on error)
+static inline int get_nvidia_target_count(Display *dpy, TARGET_ID tid)
+{
+	int num_tgts;
+	if (!XNVCTRLQueryTargetCount(dpy, translate_nvidia_target[tid], &num_tgts)) {
+		num_tgts = -1;
+	}
+
+	return num_tgts;
+}
+
+// Exit if we are unable to get targets of type tid on display dpy
+void check_nvidia_target_count(Display *dpy, TARGET_ID tid, ATTR_ID aid)
+{
+	int num_tgts = get_nvidia_target_count(dpy, tid);
+
+	if(num_tgts < 1) {
+		// Print error and exit if there's not enough targets to query
+		CRIT_ERR(NULL, NULL, "%s:"
+		"\n          Trying to query Nvidia target failed (using the propietary drivers)."
+		"\n          Are you sure they are installed correctly and a Nvidia GPU is in use?"
+		"\n          (display: %d, target_id: %d, target_count: %d, attribute_id: %d)"
+		             , __func__, dpy, tid, num_tgts, aid);
+	}
+}
 
 // Retrieve attribute value via nvidia interface
 static int get_nvidia_value(TARGET_ID tid, ATTR_ID aid)
@@ -581,8 +624,12 @@ static int get_nvidia_value(TARGET_ID tid, ATTR_ID aid)
 	Display *dpy = nvdisplay ? nvdisplay : display;
 	int value;
 
+	// Check for issues
+	check_nvidia_target_count(dpy, tid, aid);
+
 	// Query nvidia interface
 	if(!dpy || !XNVCTRLQueryTargetAttribute(dpy, translate_nvidia_target[tid], 0, 0, translate_nvidia_attribute[aid], &value)){
+		NORM_ERR("%s: Something went wrong running nvidia query (tid: %d, aid: %d)", __func__, tid, aid);
 		return -1;
 	}
 	
@@ -602,9 +649,13 @@ static char* get_nvidia_string(TARGET_ID tid, ATTR_ID aid)
 {
 	Display *dpy = nvdisplay ? nvdisplay : display;
 	char *str;
+
+	// Check for issues
+	check_nvidia_target_count(dpy, tid, aid);
 	
 	// Query nvidia interface
 	if (!dpy || !XNVCTRLQueryTargetStringAttribute(dpy, translate_nvidia_target[tid], 0, 0, translate_nvidia_attribute[aid], &str)) {
+		NORM_ERR("%s: Something went wrong running nvidia string query (tid: %d, aid: %d)", __func__, tid, aid);
 		return NULL;
 	}
 	//fprintf(stderr, "%s", str);
@@ -746,19 +797,28 @@ double get_nvidia_barval(struct text_object *obj) {
 	// Assume failure
 	value = 0;
 	
-	// Convert query_result to a percentage
+	// Convert query_result to a percentage using ((val-min)÷(max-min)×100)+0.5 if needed.
 	if (nvs != NULL) {
 		switch (nvs->attribute) {
 			case ATTR_UTILS_STRING: // one of the percentage utils (gpuutil, membwutil, videoutil and pcieutil)
 				value = get_nvidia_string_value(nvs->target, ATTR_UTILS_STRING, nvs->token, nvs->search);
 				break;
 			case ATTR_MEM_UTIL: // memutil
+			case ATTR_MEM_USED:
 				temp1 = get_nvidia_value(nvs->target, ATTR_MEM_USED);
 				temp2 = get_nvidia_value(nvs->target, ATTR_MEM_TOTAL);
 				value = ((float)temp1 * 100 / (float)temp2) + 0.5;
 				break;
+			case ATTR_MEM_FREE: // memfree
+				temp1 = get_nvidia_value(nvs->target, ATTR_MEM_USED);
+				temp2 = get_nvidia_value(nvs->target, ATTR_MEM_TOTAL);
+				value = temp2 - temp1;
+				break;
+			case ATTR_FAN_SPEED: // fanspeed: Warn user we are using fanlevel
+			NORM_ERR("%s: invalid argument specified: '%s' (using 'fanlevel' instead).",
+								nvs->command, nvs->arg);
+				// No break, continue into fanlevel
 			case ATTR_FAN_LEVEL: // fanlevel
-			case ATTR_FAN_SPEED: // TODO warn user to use fanlevel if they use fanspeed
 				value = get_nvidia_value(nvs->target, ATTR_FAN_LEVEL);
 				break;
 			case ATTR_GPU_TEMP: // gputemp (calculate out of gputempthreshold)
@@ -766,10 +826,40 @@ double get_nvidia_barval(struct text_object *obj) {
 				temp2 = get_nvidia_value(nvs->target, ATTR_GPU_TEMP_THRESHOLD);
 				value = ((float)temp1 * 100 / (float)temp2) + 0.5;
 				break;
-			// TODO: calculate gpufreq, memfreq, etc
+			case ATTR_AMBIENT_TEMP: // ambienttemp (calculate out of gputempthreshold for consistency)
-			// can use (val-min)÷(max-min)×100. Perhaps a helper function or macro
+				temp1 = get_nvidia_value(nvs->target, ATTR_AMBIENT_TEMP);
-			
+				temp2 = get_nvidia_value(nvs->target, ATTR_GPU_TEMP_THRESHOLD);
-			// TODO: throw errors if unsupported args are used
+				value = ((float)temp1 * 100 / (float)temp2) + 0.5;
+				break;
+			case ATTR_GPU_FREQ: // gpufreq (calculate out of gpufreqmax)
+				temp1 = get_nvidia_value(nvs->target, ATTR_GPU_FREQ);
+				temp2 = get_nvidia_string_value(nvs->target, ATTR_PERFMODES_STRING, (char*) "nvclockmax", SEARCH_MAX);
+				value = ((float)temp1 * 100 / (float)temp2) + 0.5;
+				break;
+			case ATTR_MEM_FREQ: // memfreq (calculate out of memfreqmax)
+				temp1 = get_nvidia_value(nvs->target, ATTR_MEM_FREQ);
+				temp2 = get_nvidia_string_value(nvs->target, ATTR_PERFMODES_STRING, (char*) "memclockmax", SEARCH_MAX);
+				value = ((float)temp1 * 100 / (float)temp2) + 0.5;
+				break;
+			case ATTR_FREQS_STRING: // mtrfreq (calculate out of memfreqmax)
+				if (nvs->token == "memTransferRate") {
+					// Just in case error for silly devs
+					CRIT_ERR(NULL, NULL, "%s: attribute is 'ATTR_FREQS_STRING' but token is not \"memTransferRate\" (arg: '%s')",
+					          nvs->command, nvs->arg);
+					return 0;
+				}
+				temp1 = get_nvidia_string_value(nvs->target, ATTR_FREQS_STRING, nvs->token, SEARCH_MAX);
+				temp2 = get_nvidia_string_value(nvs->target, ATTR_PERFMODES_STRING, (char*) "memTransferRatemax", SEARCH_MAX);
+				if (temp2 > temp1) temp1 = temp2; // extra safe here
+				value = ((float)temp1 * 100 / (float)temp2) + 0.5;
+				break;
+			case ATTR_IMAGE_QUALITY: // imagequality
+				value = get_nvidia_value(nvs->target, ATTR_IMAGE_QUALITY);
+				break;
+
+			default: // Throw error if unsupported args are used
+				CRIT_ERR(NULL, NULL, "%s: invalid argument specified: '%s'",
+				          nvs->command, nvs->arg);
 		}
 	}
 	

src/nvidia.h

@@ -32,10 +32,9 @@
 #ifndef NVIDIA_CONKY_H
 #define NVIDIA_CONKY_H
 
-int scan_nvidia_args (struct text_object *obj, const char *args, unsigned int special_t);
+int set_nvidia_query(struct text_object *, const char *, unsigned int);
-int set_nvidia_type(struct text_object *, const char *);
 void print_nvidia_value(struct text_object *, char *, int);
-double get_nvidia_barval(struct text_object *obj);
+double get_nvidia_barval(struct text_object *);
 void free_nvidia(struct text_object *);
 
 #endif