Add IPC method to select a profile

[ericonr]

This "works", in the sense that the server receives the request and applies the profile. However, it seems that it immediately triggers a "detected change" event, and the server applies the old profile again:

applying profile 'home_desk'
applying profile output 'LG HDR WFHD' on connected head 'DP-3'
applying profile output 'eDP-1' on connected head 'eDP-1'
running commands for configuration 'home_desk'
configuration for profile 'home_desk' applied <------ HERE is where I call kanshictl
loading profile only_external
applying profile 'only_external'
applying profile output 'LG HDR WFHD' on connected head 'DP-3'
applying profile output 'eDP-1' on connected head 'eDP-1'
running commands for configuration 'only_external'
configuration for profile 'only_external' applied
applying profile 'home_desk' <------- THIS starts immediately
applying profile output 'LG HDR WFHD' on connected head 'DP-3'
applying profile output 'eDP-1' on connected head 'eDP-1'
running commands for configuration 'home_desk'
configuration for profile 'home_desk' applied

I'm looking into the code to try and understand how to make this not happen, but would definitely appreciate tips on where to look :)

I hope my approach is correct, too.

[ericonr]

static void output_manager_handle_done(void *data,
        struct zwlr_output_manager_v1 *manager, uint32_t serial) {
    struct kanshi_state *state = data;
    state->serial = serial;

    //try_apply_profiles(state);
}

makes kanshi not do anything when it's started, but also makes it behave predictably when it receives kanshictl profile <> commands.

[ericonr]

Ok, I think my latest commit fixes it. I left a few questions in the commit message about the implementation.

[emersion]

The compositor will broadcast events followed by a done event when the output configuration changes. When applying a profile we trigger a configuration change.

What I originally thought of was that the profile names would act as a tie-breaker. Setting a profile name would add a new condition to match_profile, something like

if (state->selected_profile_name != NULL && profile->name != NULL &&
        strcmp(state->selected_profile_name, profile->name) != 0) {
    return false;
}

So that the normal matching logic still applies, ie.

• kanchictl switch asdf doesn't apply asdf when asdf doesn't make sense given the currently plugged in outputs
• It's possible to define multiple profiles with the same name and with a different set of outputs

To implement this, the "currently selected profile name" would need to be saved in state (in the example code above, in state->selected_profile_name).

I'm open to other ideas though.

[ericonr]

So basically you would just have me update the current name, then call try_all_profiles? I like that idea, makes the whole thing more deterministic.

Would you prefer kanshictl switch over kanshictl profile?

[emersion]

So basically you would just have me update the current name, then call try_all_profiles?

Yup!

Would you prefer kanshictl switch over kanshictl profile?

I'd prefer to pick a subcommand name that describes an action. switch may not be that great, because it may not actually switch anything. Maybe set-profile?

[ericonr]

I wonder if current and pending names wouldn't be necessary? Otherwise using a name that doesn't exist would break any further transitions.

And using set-profile even once would mean that automatic transitions would now need to also match the profile name. Maybe the manual should document that reload can be used to reset things?

[ericonr]

Maybe we should try and report an error back to kanshictl if setting the profile fails...

[emersion]

Maybe we should try and report an error back to kanshictl if setting the profile fails...

Sounds sensible.

Also adding something like kanshictl set-profile "*" to reset to "match any profile name" would be handy. (Wildcard char subject to bikeshedding, something like - might be better to avoid shell quoting.)

[ericonr]

(Wildcard char subject to bikeshedding, something like - might be better to avoid shell quoting.)

Would have to forbid profiles named -, then :p

But I like the idea.

[ericonr]

So, I've been dog fooding another version of this for a while now, which implements the idea in https://github.com/emersion/kanshi/pull/108#issuecomment-886450367

I am, however, hitting an UX issue because of it.

My config is:

profile home_desk {
    output "LG HDR WFHD" enable mode 2560x1080 position 0,0
    output eDP-1 enable scale 1.1 position 0,1080
}

profile only_external {
    output "LG HDR WFHD" enable mode 2560x1080 position 0,0
    output eDP-1 disable
}

profile only_internal {
    output "LG HDR WFHD" disable
    output eDP-1 enable scale 1.0 position 0,0
}

profile out_and_about {
    output eDP-1 enable scale 1 position 0,0
}

I start with home_desk, but will often switch to only_external; if I switch back to home_desk, and then remove the external monitor (which should trigger out_and_about), it will instead do nothing, because it can't match a profile with the name home_desk to something that only has the internal monitor. This is even worse if I leave it in only_external.

I didn't like this experience, and I think I prefer the style where set-profile is something you set once to pick a profile for that particular monitor combination, and any change in output availability will return to the normal matching logic.

That's what the code you asked about in https://github.com/emersion/kanshi/pull/108#pullrequestreview-723154254 implemented: without that, selecting a profile led to kanshi moving to it, then getting a wayland callback, and moving back to the first match in the config (as described in https://github.com/emersion/kanshi/pull/108#issue-696429472); with that, it makes sure not to override a manually selected profile (which was set via the set-profile RPC call) when the callback is called, and therefore doesn't restore the first match in the config.

[emersion]

As discussed on IRC, two potential solutions:

• Make set-profile "one-off": applies the profile with the specified name, and does nothing more. This has the downside of being fragile:
  • Requires a hack to avoid re-applying a different profile when the notification that the provided profile has been applied is sent by the compositor, but this is racy with a real hotplug.
  • Any little change in the output configuration (e.g. user executes a swaymsg output … command) will re-apply a different profile.
• Make set-profile indicate a preference: if a profile with the specified name can be found and is applicable, use it, otherwise fallback to any other profile. The preference sticks around until another set-profile command is issued.