From 5eb61e0efdbaa93189a31eecfba296b30adfd642 Mon Sep 17 00:00:00 2001 From: Maximilian Seidler <78690852+PaideiaDilemma@users.noreply.github.com> Date: Sat, 20 Apr 2024 06:09:59 +0200 Subject: [PATCH] #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 --- config.mk | 2 +- doc/CONTRIBUTING.md | 4 +-- doc/way-displays.1 | 8 +++++- doc/way-displays.1.pandoc | 8 +++++- examples/cfg.yaml | 5 ++++ inc/cfg.h | 2 ++ inc/process.h | 2 ++ src/cfg.c | 28 +++++++++++++++++++++ src/cli.c | 23 +++++++++++++++++ src/convert.c | 1 + src/info.c | 11 ++++++++ src/layout.c | 9 ++++++- src/marshalling.cpp | 14 +++++++++++ src/process.c | 25 ++++++++++++++++++ src/server.c | 13 ++++++++++ tst/GNUmakefile | 2 +- tst/marshalling/cfg-all.yaml | 1 + tst/marshalling/ipc-request-cfg-set.yaml | 1 + tst/marshalling/ipc-responses-map.yaml | 1 + tst/tst-cfg.c | 32 ++++++++++++++++++++++++ tst/tst-layout.c | 17 ++++++++++++- tst/tst-marshalling.c | 1 + tst/wrap-process.c | 5 +++- 23 files changed, 206 insertions(+), 9 deletions(-) diff --git a/config.mk b/config.mk index 86ababcf..1827b9b8 100644 --- a/config.mk +++ b/config.mk @@ -1,4 +1,4 @@ -VERSION ?= "1.10.3-SNAPSHOT" +VERSION ?= "1.11.0-SNAPSHOT" PREFIX ?= /usr/local PREFIX_ETC ?= /usr/local diff --git a/doc/CONTRIBUTING.md b/doc/CONTRIBUTING.md index a8b9ecad..0e3cf582 100644 --- a/doc/CONTRIBUTING.md +++ b/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. diff --git a/doc/way-displays.1 b/doc/way-displays.1 index 138ac2de..8854de64 100644 --- a/doc/way-displays.1 +++ b/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] diff --git a/doc/way-displays.1.pandoc b/doc/way-displays.1.pandoc index f337eef8..34ffe9a4 100644 --- a/doc/way-displays.1.pandoc +++ b/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. diff --git a/examples/cfg.yaml b/examples/cfg.yaml index f683d14c..c3092e54 100644 --- a/examples/cfg.yaml +++ b/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' diff --git a/inc/cfg.h b/inc/cfg.h index 5221eec6..12e0936b 100644 --- a/inc/cfg.h +++ b/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, diff --git a/inc/process.h b/inc/process.h index d8afaf57..645b816d 100644 --- a/inc/process.h +++ b/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); diff --git a/src/cfg.c b/src/cfg.c index d5a4679f..ad934653 100644 --- a/src/cfg.c +++ b/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); diff --git a/src/cli.c b/src/cli.c index 7d8da3ac..b3299503 100644 --- a/src/cli.c +++ b/src/cli.c @@ -41,12 +41,14 @@ void usage(FILE *stream) { " TRANSFORM <90|180|270|flipped|flipped-90|flipped-180|flipped-270>\n" " DISABLED \n" " VRR_OFF \n" + " CHANGE_SUCCESS_CMD \n" " -d, --d[elete] remove\n" " SCALE \n" " MODE \n" " TRANSFORM \n" " DISABLED \n" " VRR_OFF \n" + " CHANGE_SUCCESS_CMD \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); diff --git a/src/convert.c b/src/convert.c index 35a79deb..c61288b2 100644 --- a/src/convert.c +++ b/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", }, diff --git a/src/info.c b/src/info.c index 160c0868..913c52d4 100644 --- a/src/info.c +++ b/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) { diff --git a/src/layout.c b/src/layout.c index cb26990a..11a4f50a 100644 --- a/src/layout.c +++ b/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"); } diff --git a/src/marshalling.cpp b/src/marshalling.cpp index 05f4f09b..a6af3959 100644 --- a/src/marshalling.cpp +++ b/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().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().c_str())); + TI(cfg->laptop_display_prefix = strdup(node["LAPTOP_DISPLAY_PREFIX"].as().c_str())); TI(cfg->log_threshold = log_threshold_val(node["LOG_THRESHOLD"].as().c_str())); diff --git a/src/process.c b/src/process.c index 56b655ce..62b42ff9 100644 --- a/src/process.c +++ b/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); } diff --git a/src/server.c b/src/server.c index 67d17aec..726d69a9 100644 --- a/src/server.c +++ b/src/server.c @@ -221,6 +221,17 @@ 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); @@ -228,6 +239,8 @@ server(char *cfg_path) { // 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(); diff --git a/tst/GNUmakefile b/tst/GNUmakefile index 5da80b9d..0924c093 100644 --- a/tst/GNUmakefile +++ b/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,$\ diff --git a/tst/marshalling/cfg-all.yaml b/tst/marshalling/cfg-all.yaml index 0259d2b3..86cd669e 100644 --- a/tst/marshalling/cfg-all.yaml +++ b/tst/marshalling/cfg-all.yaml @@ -29,6 +29,7 @@ TRANSFORM: VRR_OFF: - ten - ELEVEN +CHANGE_SUCCESS_CMD: cmd LAPTOP_DISPLAY_PREFIX: ldp LOG_THRESHOLD: ERROR DISABLED: diff --git a/tst/marshalling/ipc-request-cfg-set.yaml b/tst/marshalling/ipc-request-cfg-set.yaml index 77da0068..1156a30c 100644 --- a/tst/marshalling/ipc-request-cfg-set.yaml +++ b/tst/marshalling/ipc-request-cfg-set.yaml @@ -32,6 +32,7 @@ CFG: VRR_OFF: - ten - ELEVEN + CHANGE_SUCCESS_CMD: cmd LAPTOP_DISPLAY_PREFIX: ldp LOG_THRESHOLD: ERROR DISABLED: diff --git a/tst/marshalling/ipc-responses-map.yaml b/tst/marshalling/ipc-responses-map.yaml index 676bc2c1..ba2b54bb 100644 --- a/tst/marshalling/ipc-responses-map.yaml +++ b/tst/marshalling/ipc-responses-map.yaml @@ -31,6 +31,7 @@ CFG: VRR_OFF: - ten - ELEVEN + CHANGE_SUCCESS_CMD: cmd LAPTOP_DISPLAY_PREFIX: ldp LOG_THRESHOLD: ERROR DISABLED: diff --git a/tst/tst-cfg.c b/tst/tst-cfg.c index 4f08f2b7..51b702e7 100644 --- a/tst/tst-cfg.c +++ b/tst/tst-cfg.c @@ -221,6 +221,21 @@ void merge_set__disabled(void **state) { cfg_free(merged); } +void merge_set__change_success_cmd(void **state) { + struct State *s = *state; + + s->to->change_success_cmd = strdup("to"); + s->from->change_success_cmd = strdup("from"); + + s->expected->change_success_cmd = strdup("from"); + + struct Cfg *merged = merge_set(s->to, s->from); + + assert_cfg_equal(merged, s->expected); + + cfg_free(merged); +} + void merge_del__scale(void **state) { struct State *s = *state; @@ -311,6 +326,21 @@ void merge_del__disabled(void **state) { cfg_free(merged); } +void merge_del__change_success_cmd(void **state) { + struct State *s = *state; + + s->to->change_success_cmd = strdup("to"); + s->from->change_success_cmd = strdup(""); + + s->expected->change_success_cmd = NULL; + + struct Cfg *merged = merge_del(s->to, s->from); + + assert_cfg_equal(merged, s->expected); + + cfg_free(merged); +} + void validate_fix__col(void **state) { struct State *s = *state; @@ -436,12 +466,14 @@ int main(void) { TEST(merge_set__mode), TEST(merge_set__adaptive_sync_off), TEST(merge_set__disabled), + TEST(merge_set__change_success_cmd), TEST(merge_del__scale), TEST(merge_del__mode), TEST(merge_del__transform), TEST(merge_del__adaptive_sync_off), TEST(merge_del__disabled), + TEST(merge_del__change_success_cmd), TEST(validate_fix__col), TEST(validate_fix__row), diff --git a/tst/tst-layout.c b/tst/tst-layout.c index 8a41345b..64aae2e0 100644 --- a/tst/tst-layout.c +++ b/tst/tst-layout.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -12,9 +13,10 @@ #include "global.h" #include "head.h" #include "info.h" -#include "slist.h" #include "log.h" #include "mode.h" +#include "slist.h" +#include "util.h" #include "wlr-output-management-unstable-v1.h" struct SList *order_heads(struct SList *order_name_desc, struct SList *heads); @@ -640,6 +642,18 @@ void handle_success__head_changing_mode(void **state) { assert_null(head_changing_mode); } +void handle_success__change_success_cmd(void **state) { + cfg->change_success_cmd = strdup("echo \"hi from way-displays\""); + + expect_value(__wrap_spawn_sh_cmd, command, cfg->change_success_cmd); + + handle_success(); + + assert_log(INFO, "\nExecuting CHANGE_SUCCESS_CMD:\n" + " echo \"hi from way-displays\"\n" + "\nChanges successful\n"); +} + void handle_success__ok(void **state) { handle_success(); @@ -743,6 +757,7 @@ int main(void) { TEST(handle_success__head_changing_adaptive_sync), TEST(handle_success__head_changing_adaptive_sync_fail), TEST(handle_success__head_changing_mode), + TEST(handle_success__change_success_cmd), TEST(handle_success__ok), TEST(handle_failure__mode), diff --git a/tst/tst-marshalling.c b/tst/tst-marshalling.c index 0c8602dc..4f2937df 100644 --- a/tst/tst-marshalling.c +++ b/tst/tst-marshalling.c @@ -66,6 +66,7 @@ struct Cfg *cfg_all(void) { cfg->auto_scale_min = 0.5f; cfg->auto_scale_max = 2.5f; + cfg->change_success_cmd = strdup("cmd"); cfg->laptop_display_prefix = strdup("ldp"); slist_append(&cfg->order_name_desc, strdup("one")); diff --git a/tst/wrap-process.c b/tst/wrap-process.c index e2ed7611..22a7ab4f 100644 --- a/tst/wrap-process.c +++ b/tst/wrap-process.c @@ -2,6 +2,10 @@ #include +void __wrap_spawn_sh_cmd(const char * const command) { + check_expected(command); +} + void __wrap_wd_exit(int __status) { check_expected(__status); } @@ -9,4 +13,3 @@ void __wrap_wd_exit(int __status) { void __wrap_wd_exit_message(int __status) { check_expected(__status); } -