Fix: The monitor's screen size is incorrect.

[GongHeng2017]

-- For some monitors, detailed size information is not recorded in the EDID file;
in such cases, obtaining the basic size information is sufficient.
-- Therefore, remove the logic that retrieves the detailed size information again.

Log: fix issue
Bug: https://pms.uniontech.com/bug-view-354275.html

Summary by Sourcery

Correct monitor screen size parsing from EDID data and improve error handling for invalid EDID blocks.

Bug Fixes:

• Fix incorrect monitor width and height calculation by properly handling detailed timing bytes in both little-endian and big-endian EDID formats and falling back to basic size when they diverge.
• Skip and log EDID entries that fail validation instead of silently proceeding.

Enhancements:

• Clarify EDID validation error messages by distinguishing little-endian and big-endian failure modes.

[sourcery-ai]

Reviewer's Guide

Refines EDID parsing for monitor size and error handling by clarifying endian-specific validation messages, correcting Detailed Timing parsing for both endian modes, removing redundant screen size recomputation logic, and making EDID parsing failures in CommonTools visible and non-fatal for the overall parsing loop.

Sequence diagram for EDID parsing with improved error handling

sequenceDiagram
    participant CommonTools
    participant EDIDParser
    participant Logger

    CommonTools->>CommonTools: parseEDID(allEDIDS, input, isHW)
    loop for each edidStr in allEDIDS
        CommonTools->>EDIDParser: setEdid(edidStr, errorMsg, separator, needLittleEndian)
        alt setEdid returns false
            EDIDParser-->>CommonTools: false, errorMsg
            CommonTools->>Logger: qCritical(errorMsg)
            CommonTools->>CommonTools: continue to next edidStr
        else setEdid returns true
            EDIDParser-->>CommonTools: true
            CommonTools->>CommonTools: build mapInfo from EDIDParser
        end
    end

Loading

Updated class diagram for EDIDParser and CommonTools

classDiagram
    class EDIDParser {
        -bool m_LittleEndianMode
        -int m_Width
        -int m_Height
        +bool setEdid(QString edid, QString errorMsg, QString separator, bool needLittleEndian)
        +void parseReleaseDate()
        +void parseScreenSize()
        +QString vendor()
        -QString getBytes(int lineIndex, int byteIndex)
        -QString hexToDec(QString hexValue)
    }

    class CommonTools {
        +static void parseEDID(QStringList allEDIDS, QString input, bool isHW)
    }

    CommonTools ..> EDIDParser : uses

    class Common {
        <<static>> int specialComType
        <<static>> int kSpecialType7
    }

    EDIDParser ..> Common : reads

Loading

File-Level Changes

Change	Details	Files
Clarify EDID header validation errors for little-endian and big-endian modes.	Update error message when little-endian EDID header does not match expected prefix to explicitly mention little-endian mode Update error message when big-endian EDID header does not match expected prefix to explicitly mention big-endian mode	deepin-devicemanager/src/Tool/EDIDParser.cpp
Adjust screen size parsing from Detailed Timing to correctly handle endian-specific byte ordering and simplify the logic.	Document the Detailed Timing bytes (66-68) layout used for screen size extraction In little-endian mode, compute width and height using low bytes at positions 2 and 3 and the high-nibble byte at position 4 In big-endian mode, swap the byte positions to match the actual width/height layout, using positions 3, 2, and 5 respectively Remove the older alternative recomputation of width and height using s66/s67/s68 to avoid redundant or conflicting size calculations	deepin-devicemanager/src/Tool/EDIDParser.cpp
Improve EDID parsing robustness and logging in CommonTools.	Check the boolean return value of EDIDParser::setEdid when parsing each EDID string On failure, log the specific error message via qCritical Skip the current EDID entry on parse failure and continue with the rest	deepin-devicemanager/src/Tool/commontools.cpp

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[github-actions]

Note

[静态代码检查]

• 敏感词检查失败, 检测到1个文件存在敏感词

详情

{
    "deepin-devicemanager/src/Tool/commontools.cpp": [
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 165,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 169,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://drive-pre.uniontech.com/api/v1/drive/search\";",
            "line_number": 171,
            "rule": "S35",
            "reason": "Url link | 0e010283c1"
        }
    ]
}

[sourcery-ai]

Hey - I've left some high level feedback:

• In EDIDParser::parseScreenSize, the new big-endian branch mixes variable names and comments (e.g., tmpw is documented as the original height byte and tmph as the original width byte); aligning names and comments with their actual semantic meaning would make this logic much easier to maintain and verify.
• The new qCritical() log in CommonTools::parseEDID will fire for every invalid EDID; if this function is called on large batches of monitors, consider lowering the log level or adding some deduplication/context so logs don’t become excessively noisy.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `EDIDParser::parseScreenSize`, the new big-endian branch mixes variable names and comments (e.g., `tmpw` is documented as the original height byte and `tmph` as the original width byte); aligning names and comments with their actual semantic meaning would make this logic much easier to maintain and verify.
- The new `qCritical()` log in `CommonTools::parseEDID` will fire for every invalid EDID; if this function is called on large batches of monitors, consider lowering the log level or adding some deduplication/context so logs don’t become excessively noisy.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[github-actions]

Note

[静态代码检查]

• 敏感词检查失败, 检测到1个文件存在敏感词

详情

{
    "deepin-devicemanager/src/Tool/commontools.cpp": [
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 165,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://driver.uniontech.com/api/v1/drive/search\";",
            "line_number": 169,
            "rule": "S35",
            "reason": "Url link | 162cef9d4a"
        },
        {
            "line": "        return \"https://drive-pre.uniontech.com/api/v1/drive/search\";",
            "line_number": 171,
            "rule": "S35",
            "reason": "Url link | 0e010283c1"
        }
    ]
}

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, max-lvs

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[GongHeng2017]

/merge

[deepin-bot]

This pr cannot be merged! (status: unstable)

[GongHeng2017]

/forcemerge

[deepin-bot]

This pr force merged! (status: unstable)