#65 add ON_CHANGE_CMD to run a shell command in handle_success (#162)

* add spawn_async function

* add ON_CHANGE_CMD config

* spawn on_change_cmd in handle_success

* adapt marshalling tests

* isolate the spawed process a bit more

* ignore adaptive sync changes

Can't think of a use case to run a command when vrr changes

* add a test for the change command

* add ON_CHANGE_CMD to cfg.yaml

* sanitary fixes

* wrap spawn_async and simplify the on_change_cmd test

Now we are not testing if the command actually gets executed.

* fix mem leak due to creating the default config twice

* rename to CHANGE_SUCCESS_CMD

* add cli set and delete for CHANGE_SUCCESS_CMD

* add CHANGE_SUCCESS_CMD to print_cfg and print_cfg_commands

* add cfg merge tests for change_success_cmd

* revision spawn command

- only fork once
- rename from spawn_async to spawn_sh_cmd

* add CHANGE_SUCCESS_CMD to cli usage

* add CHANGE_SUCCESS_CMD to man

* prepare for next minor release

* setup signal handlers in the server to avoid zombie processes

Thanks to dwm :)

* add compound command example, unify man and cfg.yaml

---------

Co-authored-by: Alexander Courtis <[email protected]>

config.mk

@@ -1,4 +1,4 @@
-VERSION ?= "1.10.3-SNAPSHOT"
+VERSION ?= "1.11.0-SNAPSHOT"
 
 PREFIX ?= /usr/local
 PREFIX_ETC ?= /usr/local

doc/CONTRIBUTING.md

@@ -69,7 +69,7 @@ See all violations:
 
 ## Documentation
 
-Please update `README.md` and `doc/configuration.md`.
+Please update `README.md`.
 
 Please update the man page:
 * update `way-displays.1.pandoc`
@@ -83,7 +83,7 @@ Please match the style of the surrounding code and obey `.editorconfig`. Default
 
 ## Adding Options
 
-Please add the option to `config.yaml` with a descriptive comment.
+Please add the option to `cfg.yaml` with a descriptive comment.
 
 Please add a command line option and update the usage message.
 

doc/way-displays.1

@@ -14,7 +14,7 @@
 . ftr VB CB
 . ftr VBI CBI
 .\}
-.TH "WAY-DISPLAYS" "1" "2023/08/29" "way-displays" "User Manuals"
+.TH "WAY-DISPLAYS" "1" "2024/04/20" "way-displays" "User Manuals"
 .hy
 .SH NAME
 .PP
@@ -140,6 +140,9 @@ Disable VRR for a display.
 .TP
 \f[V]DISABLED\f[R] <\f[I]name\f[R]>
 Disable a display.
+.TP
+\f[V]CHANGE_SUCCESS_CMD\f[R] <\f[I]shell command\f[R]>
+Sets a \f[V]/bin/sh\f[R] command to be executed when display configurations are successfully changed e.g.\ \f[V]bell ; notify-send way-displays \[dq]change success!\[dq]\f[R]
 .RE
 .TP
 \f[V]-d\f[R], \f[V]--d[elete]\f[R]
@@ -160,6 +163,9 @@ Enable VRR for a display.
 .TP
 \f[V]DISABLED\f[R] <\f[I]name\f[R]>
 Enable a display.
+.TP
+\f[V]CHANGE_SUCCESS_CMD\f[R] <\f[I]shell command\f[R]>
+Remove command on display configuration success.
 .RE
 .TP
 \f[V]-w\f[R] | \f[V]--w[rite]\f[R]

doc/way-displays.1.pandoc

@@ -1,6 +1,6 @@
 % WAY-DISPLAYS(1) way-displays | User Manuals
 % Alexander Courtis
-% 2023/08/29
+% 2024/04/20
 
 # NAME
 
@@ -112,6 +112,9 @@ The user must be a member of the `input` group.
 	`DISABLED` <*name*>
 	: Disable a display.
 
+	`CHANGE_SUCCESS_CMD` <*shell command*>
+    : Sets a `/bin/sh` command to be executed when display configurations are successfully changed e.g. `bell ; notify-send way-displays "Monitor changed"`
+
 `-d`, `--d[elete]`
 : Remove an existing setting.
 
@@ -130,6 +133,9 @@ The user must be a member of the `input` group.
 	`DISABLED` <*name*>
 	: Enable a display.
 
+    `CHANGE_SUCCESS_CMD` <*shell command*>
+    : Remove command on display configuration success.
+
 `-w` | `--w[rite]`
 : Write active configuration to cfg.yaml; removes any whitespace or comments.
 

examples/cfg.yaml

@@ -67,6 +67,11 @@ VRR_OFF:
   # - '!.*my monitor.*'
 
 
+# Sets a `/bin/sh` command to be executed when display configurations are successfully changed.
+# NOTE: Depending on your compositor this could get executed multiple times when
+# a change happens. Especially likely on a (dis-)connect.
+#CHANGE_SUCCESS_CMD: 'bell ; notify-send way-displays "Monitor changed"'
+
 # Laptop displays usually start with eDP e.g. eDP-1. This may be overridden if
 # your laptop is different.
 #LAPTOP_DISPLAY_PREFIX: 'eDP'

inc/cfg.h

@@ -61,6 +61,7 @@ struct Cfg {
 
 	bool updated;
 
+	char *change_success_cmd;
 	char *laptop_display_prefix;
 	struct SList *order_name_desc;
 	enum Arrange arrange;
@@ -89,6 +90,7 @@ enum CfgElement {
 	MODE,
 	TRANSFORM,
 	VRR_OFF,
+	CHANGE_SUCCESS_CMD,
 	LAPTOP_DISPLAY_PREFIX,
 	MAX_PREFERRED_REFRESH,
 	LOG_THRESHOLD,

inc/process.h

@@ -9,6 +9,8 @@ pid_t pid_active_server(void);
 
 void pid_file_create(void);
 
+void spawn_sh_cmd(const char * const command);
+
 // exit; caller should return afterwards
 void wd_exit(int __status);
 

src/cfg.c

@@ -266,6 +266,11 @@ struct Cfg *clone_cfg(struct Cfg *from) {
 		slist_append(&to->adaptive_sync_off_name_desc, strdup((char*)i->val));
 	}
 
+	// CHANGE_SUCCESS_CMD
+	if (from->change_success_cmd) {
+		to->change_success_cmd = strdup(from->change_success_cmd);
+	}
+
 	// LAPTOP_DISPLAY_PREFIX
 	if (from->laptop_display_prefix) {
 		to->laptop_display_prefix = strdup(from->laptop_display_prefix);
@@ -345,6 +350,13 @@ bool cfg_equal(struct Cfg *a, struct Cfg *b) {
 		return false;
 	}
 
+	// CHANGE_SUCCESS_CMD
+	char *ao = a->change_success_cmd;
+	char *bo = b->change_success_cmd;
+	if ((ao && !bo) || (!ao && bo) || (ao && bo && strcmp(ao, bo) != 0)) {
+		return false;
+	}
+
 	// LAPTOP_DISPLAY_PREFIX
 	char *al = a->laptop_display_prefix;
 	char *bl = b->laptop_display_prefix;
@@ -643,6 +655,14 @@ struct Cfg *merge_set(struct Cfg *to, struct Cfg *from) {
 		}
 	}
 
+	// CHANGE_SUCCESS_CMD
+	if (from->change_success_cmd) {
+		if (merged->change_success_cmd) {
+			free(merged->change_success_cmd);
+		}
+		merged->change_success_cmd = strdup(from->change_success_cmd);
+	}
+
 	return merged;
 }
 
@@ -680,6 +700,12 @@ struct Cfg *merge_del(struct Cfg *to, struct Cfg *from) {
 		slist_remove_all_free(&merged->disabled_name_desc, slist_predicate_strcmp, i->val, NULL);
 	}
 
+	// CHANGE_SUCCESS_CMD
+	if (from->change_success_cmd && strlen(from->change_success_cmd) == 0) {
+		free(merged->change_success_cmd);
+		merged->change_success_cmd = NULL;
+	}
+
 	return merged;
 }
 
@@ -845,6 +871,8 @@ void cfg_free(struct Cfg *cfg) {
 
 	cfg_free_paths(cfg);
 
+	free(cfg->change_success_cmd);
+
 	free(cfg->laptop_display_prefix);
 
 	slist_free_vals(&cfg->order_name_desc, NULL);

src/cli.c

@@ -41,12 +41,14 @@ void usage(FILE *stream) {
 		"     TRANSFORM <name> <90|180|270|flipped|flipped-90|flipped-180|flipped-270>\n"
 		"     DISABLED <name>\n"
 		"     VRR_OFF <name>\n"
+		"     CHANGE_SUCCESS_CMD <shell command>\n"
 		"  -d, --d[elete]  remove\n"
 		"     SCALE <name>\n"
 		"     MODE <name>\n"
 		"     TRANSFORM <name>\n"
 		"     DISABLED <name>\n"
 		"     VRR_OFF <name>\n"
+		"     CHANGE_SUCCESS_CMD <shell command>\n"
 		;
 	fprintf(stream, "%s", mesg);
 }
@@ -156,6 +158,19 @@ struct Cfg *parse_element(enum IpcCommand command, enum CfgElement element, int
 			}
 			parsed = true;
 			break;
+		case CHANGE_SUCCESS_CMD:
+			switch (command) {
+				case CFG_SET:
+					cfg->change_success_cmd = strdup(argv[optind]);
+					parsed = true;
+					break;
+				case CFG_DEL:
+					cfg->change_success_cmd = strdup("");
+					parsed = true;
+					break;
+				default:
+					break;
+			}
 		default:
 			break;
 	}
@@ -226,6 +241,7 @@ struct IpcRequest *parse_set(int argc, char **argv) {
 		case AUTO_SCALE:
 		case DISABLED:
 		case VRR_OFF:
+		case CHANGE_SUCCESS_CMD:
 			if (optind + 1 != argc) {
 				log_error("%s requires one argument", cfg_element_name(element));
 				wd_exit(EXIT_FAILURE);
@@ -266,6 +282,13 @@ struct IpcRequest *parse_del(int argc, char **argv) {
 				return NULL;
 			}
 			break;
+		case CHANGE_SUCCESS_CMD:
+			if (optind != argc) {
+				log_error("%s takes no arguments", cfg_element_name(element));
+				wd_exit(EXIT_FAILURE);
+				return NULL;
+			}
+			break;
 		default:
 			log_error("invalid %s: %s", ipc_command_friendly(CFG_DEL), element ? cfg_element_name(element) : optarg);
 			wd_exit(EXIT_FAILURE);

src/convert.c

@@ -24,6 +24,7 @@ static struct NameVal cfg_elements[] = {
 	{ .val = SCALE,                 .name = "SCALE",                 },
 	{ .val = MODE,                  .name = "MODE",                  },
 	{ .val = VRR_OFF,               .name = "VRR_OFF",               },
+	{ .val = CHANGE_SUCCESS_CMD,    .name = "CHANGE_SUCCESS_CMD"     },
 	{ .val = LAPTOP_DISPLAY_PREFIX, .name = "LAPTOP_DISPLAY_PREFIX", },
 	{ .val = MAX_PREFERRED_REFRESH, .name = "MAX_PREFERRED_REFRESH", },
 	{ .val = TRANSFORM,             .name = "TRANSFORM",             },

src/info.c

@@ -205,6 +205,11 @@ void print_cfg(enum LogThreshold t, struct Cfg *cfg, bool del) {
 		}
 	}
 
+	if (cfg->change_success_cmd) {
+		log_(t, "  Change success command:");
+		log_(t, "    %s", cfg->change_success_cmd);
+	}
+
 	if (cfg->laptop_display_prefix) {
 		log_(t, "  Laptop display prefix: %s", cfg->laptop_display_prefix);
 	}
@@ -293,6 +298,12 @@ void print_cfg_commands(enum LogThreshold t, struct Cfg *cfg) {
 		print_newline(t, &newline);
 		log_(t, "way-displays -s VRR_OFF '%s'", (char*)i->val);
 	}
+
+	newline = true;
+	if (cfg->change_success_cmd) {
+		print_newline(t, &newline);
+		log_(t, "way-displays -s CHANGE_SUCCESS_CMD '%s'", cfg->change_success_cmd);
+	}
 }
 
 void print_head_current(enum LogThreshold t, struct Head *head) {

src/layout.c

@@ -333,7 +333,7 @@ void report_adaptive_sync_fail(struct Head *head) {
 void handle_success(void) {
 	if (head_changing_mode) {
 
-		// succesful mode change is not always reported
+		// successful mode change is not always reported
 		head_changing_mode->current.mode = head_changing_mode->desired.mode;
 
 		head_changing_mode = NULL;
@@ -351,6 +351,13 @@ void handle_success(void) {
 		}
 	}
 
+	if (!head_changing_adaptive_sync && cfg->change_success_cmd) {
+		log_info("\nExecuting CHANGE_SUCCESS_CMD:");
+		log_info("  %s", cfg->change_success_cmd);
+
+		spawn_sh_cmd(cfg->change_success_cmd);
+	}
+
 	log_info("\nChanges successful");
 }
 

src/marshalling.cpp

@@ -262,6 +262,10 @@ YAML::Emitter& operator << (YAML::Emitter& e, struct Cfg& cfg) {
 		e << YAML::EndSeq;												// MAX_PREFERRED_REFRESH
 	}
 
+	if (cfg.change_success_cmd) {
+		e << YAML::Key << "CHANGE_SUCCESS_CMD" << YAML::Value << cfg.change_success_cmd;
+	}
+
 	if (cfg.laptop_display_prefix) {
 		e << YAML::Key << "LAPTOP_DISPLAY_PREFIX" << YAML::Value << cfg.laptop_display_prefix;
 	}
@@ -373,6 +377,14 @@ struct CfgValidated*& operator << (struct CfgValidated*& cfg_validated, const YA
 		}
 	}
 
+	if (node["CHANGE_SUCCESS_CMD"]) {
+		if (cfg->change_success_cmd) {
+			free(cfg->change_success_cmd);
+		}
+
+		cfg->change_success_cmd = strdup(node["CHANGE_SUCCESS_CMD"].as<std::string>().c_str());
+	}
+
 	if (node["LAPTOP_DISPLAY_PREFIX"]) {
 		if (cfg->laptop_display_prefix) {
 			free(cfg->laptop_display_prefix);
@@ -709,6 +721,8 @@ struct Cfg*& operator << (struct Cfg*& cfg, const YAML::Node& node) {
 		}
 	}
 
+	TI(cfg->change_success_cmd = strdup(node["CHANGE_SUCCESS_CMD"].as<std::string>().c_str()));
+
 	TI(cfg->laptop_display_prefix = strdup(node["LAPTOP_DISPLAY_PREFIX"].as<std::string>().c_str()));
 
 	TI(cfg->log_threshold = log_threshold_val(node["LOG_THRESHOLD"].as<std::string>().c_str()));

src/process.c

@@ -103,6 +103,31 @@ void pid_file_create(void) {
 	free(path);
 }
 
+void spawn_sh_cmd(const char * const command) {
+	pid_t pid = fork();
+	if (pid < 0) {
+		log_error_errno("\nfailed to fork");
+		return;
+	}
+
+	if (pid == 0) {
+		struct sigaction sa;
+
+		setsid();
+		sigemptyset(&sa.sa_mask);
+		// reset signals to the default
+		sa.sa_flags = 0;
+		sa.sa_handler = SIG_DFL;
+		sigaction(SIGCHLD, &sa, NULL);
+
+		// execute command in the child process
+		execl("/bin/sh", "/bin/sh", "-c", command, (char *)NULL);
+		log_error_errno("\nfailed to execute /bin/sh");
+		// exit the child process in case the exec fails
+		exit(-1);
+	}
+}
+
 void wd_exit(int __status) {
 	exit(__status);
 }

src/server.c

@@ -221,13 +221,26 @@ int loop(void) {
 	}
 }
 
+void setup_signal_handlers(void) {
+	struct sigaction sa;
+
+	// don't transform child processes into zombies and don't handle SIGCHLD.
+	sigemptyset(&sa.sa_mask);
+	sa.sa_flags = SA_NOCLDSTOP | SA_NOCLDWAIT | SA_RESTART;
+	sa.sa_handler = SIG_DFL;
+	sigaction(SIGCHLD, &sa, NULL);
+}
+
+
 int
 server(char *cfg_path) {
 	log_set_times(true);
 
 	// only one instance
 	pid_file_create();
 
+	setup_signal_handlers();
+
 	// don't log anything until cfg log level is known
 	log_capture_start();
 	log_suppress_start();

tst/GNUmakefile

@@ -14,7 +14,7 @@ WRAPS_COMMON = -Wl,$\
 	--wrap=log_set_threshold,$\
 	--wrap=log_,--wrap=log_error,--wrap=log_warn,--wrap=log_info,--wrap=log_debug,--wrap=log_error_errno,$\
 	--wrap=print_head,--wrap=print_mode,$\
-	--wrap=wd_exit,--wrap=wd_exit_message
+	--wrap=spawn_sh_cmd,--wrap=wd_exit,--wrap=wd_exit_message
 
 tst-head: WRAPS=,$\
 	--wrap=mode_dpi,$\