Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prefer serial no over serial text #410

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BronzeDeer
Copy link

@BronzeDeer BronzeDeer commented Feb 3, 2025

Fixes #406

The current serial based matching behaviour was specified in d9b1953. The commit message specifies that matching on serial number should be preferred over constructing a text based fingerprint from timing information (which matches legacy behaviour of autorandr).

However, the actual code has the conditional reversed, which means that the new behaviour was never employed. This means the matching behaviour is still suffering from the previous issues like changing edid data when the same monitor switches interface etc (DP to HDMI for example) or the port on the gpu changes and now has different capabilities etc.

This commit switches the conditional around, using serial no. if it is not zero, using the legacy serial_text if available and otherwise setting serial to None

The current serial based matching behaviour was specified in d9b1953.
The commit message specifies that matching on serial number should be
preferred over constructing a text based fingerprint from timing
information (which matches legacy behaviour of autorandr).

However, the actual code has the conditional reversed however, which
means that the new behaviour was never employed. This means the matching
behaviour is still suffering from the previous issues like changing edid
data when the same monitor switches interface etc (DP to HDMI for
example) or the port on the gpu changes and now has different
capabilities etc.

This commit switches the conditional around, using serial no. if it is
not zero, using the legacy serial_text if available and otherwise
setting serial to None
@phillipberndt
Copy link
Owner

Sounds reasonable, thanks. Let's see if someone else using this feature and following the repo complains 🤔 If not, then I think merging and seeing what happens is the best path forward.

What's WIP?

@BronzeDeer BronzeDeer changed the title [WIP] fix: prefer serial no over serial text fix: prefer serial no over serial text Feb 3, 2025
@BronzeDeer
Copy link
Author

Sounds reasonable, thanks. Let's see if someone else using this feature and following the repo complains 🤔 If not, then I think merging and seeing what happens is the best path forward.

What's WIP?

Thank you for taking such a quick look! I just flagged the PR as WIP (Work in progress) as I am used to gitlab automatically putting a PR into draft mode when it contains WIP: or [WIP]. I put it into draft because I had not been able to test it before uploading (or atleast it was easier to override my nix config if it was a different revision in the same repo).

This change fixed my case where I was using two monitors of the same modell which showed up with the same timing informations but ultimately could be differentiated by serial no.

Since this is now tested sucessfully on my end I removed the "WIP" tag and consider it mergeable from my side

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--match-edid matches different edids to same name
2 participants