Add feature to match display's native resolution #342
Conversation
autorandr.py
Outdated
| horz = (edid[offset + 8] << 8) + edid[offset + 7] | ||
| if horz == 0: | ||
| return None | ||
|
|
||
| vert = (edid[offset + 10] << 8) + edid[offset + 9] | ||
|
|
||
| return (horz, vert) | ||
|
|
||
|
|
||
| def _read_resolution_from_displayid_block_0x0c(edid, offset): | ||
| payload_length = edid[offset + 2] | ||
|
|
||
| if payload_length != 13: | ||
| return None | ||
|
|
||
| horz = (edid[offset + 6] << 8) + edid[offset + 5] + 1 | ||
| if horz == 0: | ||
| return None | ||
|
|
||
| vert = (edid[offset + 8] << 8) + edid[offset + 7] + 1 | ||
|
|
||
| return (horz, vert) | ||
|
|
||
|
|
||
| def _read_resolution_from_displayid_block_0x21(edid, offset): | ||
| payload_length = edid[offset + 2] | ||
|
|
||
| if payload_length != 29: | ||
| return None | ||
|
|
||
| horz = (edid[offset + 8] << 8) + edid[offset + 7] | ||
| if horz == 0: | ||
| return None | ||
|
|
||
| vert = (edid[offset + 10] << 8) + edid[offset + 9] | ||
|
|
||
| return (horz, vert) | ||
|
|
||
|
|
||
| def _read_resolution_from_displayid_block(edid, offset): | ||
| tag = edid[offset] | ||
| payload_length = edid[offset + 2] | ||
|
|
||
| if tag == 0x01: | ||
| res = _read_resolution_from_displayid_block_0x01(edid, offset) | ||
| elif tag == 0x0c: | ||
| res = _read_resolution_from_displayid_block_0x0c(edid, offset) | ||
| elif tag == 0x21: | ||
| res = _read_resolution_from_displayid_block_0x21(edid, offset) | ||
| else: | ||
| res = None | ||
|
|
||
| # Add header length of 3 | ||
| return payload_length + 3, res | ||
|
|
||
|
|
||
| def _read_resolutions_from_displayid_block(edid, offset): | ||
| # DisplayID length has a maximum of 121 | ||
| total_length = min(edid[offset + 2], 121) | ||
| base = offset + 5 | ||
|
|
||
| resolutions = [] | ||
| detail = 0 | ||
| while detail < total_length: | ||
| block_length, resolution = \ | ||
| _read_resolution_from_displayid_block(edid, base + detail) | ||
| if resolution: | ||
| resolutions.append(resolution) | ||
|
|
||
| detail += block_length | ||
|
|
||
| return resolutions | ||
|
|
||
|
|
||
| def _read_resolutions_from_block(edid, offset): | ||
| block_type = edid[offset] | ||
|
|
||
| if block_type == 0x00: | ||
| return _read_resolutions_from_base_block(edid, offset) | ||
| if block_type == 0x02: | ||
| return _read_resolutions_from_cta_block(edid, offset) | ||
| if block_type == 0x70: | ||
| return _read_resolutions_from_displayid_block(edid, offset) | ||
|
|
||
| return [] | ||
|
|
||
|
|
||
| def read_native_resolutions(edid): | ||
| """Given an edid hex string, return a set of all the native resolutions | ||
| (horizontal, vertical) defined in the EDID""" | ||
|
|
||
| edid_page_size = 128 | ||
|
|
||
| edid_bytearray = bytearray.fromhex(edid) | ||
|
|
||
| native_resolutions = set() | ||
| for offset in range(0, len(edid_bytearray), edid_page_size): | ||
| native_resolutions.update(_read_resolutions_from_block(edid_bytearray, offset)) | ||
|
|
||
| return native_resolutions |
There was a problem hiding this comment.
The one thing I don't like about this PR is how we need 153 lines of code for parsing some hex data, introducing top level functions with non-intuitive names like _read_resolution_from_displayid_block_0x01. How can we simplify this?
At the very least, I think defining the helper functions in the body of read_native_resolutions might help here. But I think you could also simplify the implementation, e.g. the _read_resolution_from_displayid_block_XXX functions can easily be merged into a single function that uses a different offset for type 0x0c. Likewise, _read_resolutions_from_base_blockand thecta` equivalent look quite similar and might be mergeable.
|
Thanks for your review! I'll refactor it and update this PR |
Accepts new setup definitions like DP-1 native_resolution=3840x2160 eDP-1 00ffffff... To match any monitors attached as DP-1 with native resolution of 3840x2160
|
I've merged some of the functions as you suggested and updated the PR. I think further inlining of functions or making functions inner functions will probably not improve readability. Hope this works for you! |
Accepts new setup definitions like
DP-1 native_resolution=3840x2160
eDP-1 00ffffff...
To match any monitors attached as DP-1 with native resolution of 3840x2160
Fixes #240