Add powered to ConnectorSavedState#366
Conversation
| if let Some(saved_state) = state | ||
| .pinnacle | ||
| .config | ||
| .connector_saved_states | ||
| .get_mut(&output_name) | ||
| { | ||
| saved_state.loc.x = x; | ||
| saved_state.loc.y = y; | ||
| } else { | ||
| state.pinnacle.config.connector_saved_states.insert( | ||
| output_name.clone(), | ||
| ConnectorSavedState { | ||
| loc: (x, y).into(), | ||
| ..Default::default() | ||
| }, | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Are you sure this is dead code ?
As I understand it, the iftrue branch allows you to set the location of a previously connected output, and the else branch allows you to set the location of an output before it's plugged.
This can be useful on e.g. a laptop if you already know which port correspond to which name.
There was a problem hiding this comment.
Sorry, what I meant is, that it is not dead code as in it never gets called. It is dead code in a way that it doesn't change anything. As far as I can tell there are only two ways an output can be disabled and that is via the set_enabled(false) and remove_output. Both of these completely overwrite what is stored in connector_saved_states. So no matter what was there it will be overwritten when you actually remove the output.
There was a problem hiding this comment.
set_output_enabled is called from the OutputManagementProtocol handlers, and remove_output is called from udev backend (src/backend/udev.rs:1106).
At least remove_output checks that the output object is defined before calling remove_output, meaning the saved state is not overriden.
Beside that, this state is also used when an output is connected (or connected again). Meaning, even if remove_output or set_output_enabled did overwrite the saved state, set_loc on a disconnected output would still allow to set the output location ahead of a future re-connection, so I'm still not convinced this is dead code (in the sense of its side effect will always be overwritten and can never be useful)
There was a problem hiding this comment.
There is one case that I did miss. With this, you can use set_loc for output that has never been seen before or is currently disconnected. As there is no check that the output in set_loc is actually valid. You still probably couldn't actually use it like that as it is only called on OutputHandle via API, so you would need to have valid output, but I guess in that case it still makes sense to keep it.
There was a problem hiding this comment.
I put it back and changed powered to Option<bool> so now it will be None if you use set_loc for non-existent output.
anriha
force-pushed
the
save_powered_enabled
branch
from
c5427ae to
596d6ce
Compare
Ottatop
force-pushed
the
save_powered_enabled
branch
from
596d6ce to
7096f25
Compare
3 participants
This PR introduces the powered field to
ConnectorSavedStateto correctly handle monitors that automatically try to reconnect after being powered off.When an output is deactivated, its last known powered state is now saved. Upon reconnection, this saved state is reapplied, ensuring that an output intended to be off remains off. This resolves power-looping issues as discussed in #362.
Additionally, this change removes the creation ofConnectorSavedStatefrom theset_locAPI call. As far as I can tell there is no situation where that would be used, as it would always be overwritten by eitherset_enableorremove_output.