Document i1Display3 per-unit spectral correction limitation#2
Merged
Conversation
The native i1Display3 driver uses hardcoded correction matrices instead of reading per-unit spectral sensitivity curves from the device EEPROM. Unit-to-unit variance is significant and the hardcoded matrices can produce incorrect measurements, especially on OLED and wide-gamut displays. Changes: - Added detailed comment in i1display.py explaining the issue and fix path - Marked correction matrices as approximate, per-unit EEPROM needed - Updated README to disclose the limitation and recommend ArgyllCMS backend - Created issue #1 to track the proper fix The sensorless calibration mode is unaffected. Thanks to dogelition for identifying this issue. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Native fix for issue #1: i1Display3 per-unit spectral correction. The driver now reads calibration matrices from the connected device's EEPROM instead of using hardcoded constants. Each i1Display3 unit stores unique calibration data for 9 display technologies. New implementation: - _read_eeprom(offset, length): Reads raw bytes via USB HID CMD_RD_EE (0x0800) with chunked reads for requests > 58 bytes - _parse_cal_matrix(raw): Parses 3x3 big-endian IEEE 754 double matrix (72 bytes) - _read_calibration(): Reads OLED matrix from device EEPROM at offset 0x199C, validates values, falls back to approximate constants only if read fails - set_display_type(type): Loads any of the 9 calibration matrices from this device (OLED, WhiteLED, CCFL, WideGamutCCFL, RGBLED, etc.) - _cal_source field tracks provenance: "device_eeprom" vs "fallback_approximate" CAL_OFFSETS maps all 9 stored calibration blocks: Ambient (0x0058), CCFL (0x04D8), WideGamutCCFL (0x0958), WhiteLED (0x0DD8), RGBLED (0x1258), OLED (0x191C), RGPhosphorBlueLED (0x1B58), WideGamutLEDPA2 (0x1FD8), Last (0x2458) The fallback matrix (from NEC MDSVSENSOR3) is preserved for devices where EEPROM reading fails, but is now clearly marked as approximate. 297 tests pass. No regressions. Addresses #1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
set_display_correction() now attempts to read per-unit calibration from the native i1Display3 EEPROM driver before falling back to hardcoded approximate matrices. Flow: 1. Open native i1D3 driver 2. Read per-unit EEPROM matrix for the requested display type 3. Use the device-specific matrix for correction 4. If native driver unavailable or EEPROM read fails, fall back to approximate constants (with clear provenance) This replaces the workaround (hardcoded matrices for all units) with the native fix (per-unit EEPROM data from each specific device). 297 tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaced limitation notice with description of the actual capability: native USB HID driver reads per-unit calibration from each device's EEPROM. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sensorless mode is unaffected.
Thanks to dogelition for identifying this via community feedback.
Test plan