Skip to content

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Jan 26, 2026

Added range validation for wheel speed configuration values to ensure they stay within the valid range of 1-100. Previously, the code only had a comment indicating the valid range but no actual validation. Now if the configured speed is below 1, it will be set to 1, and if above 100, it will be set to 100. This prevents invalid speed values from being used and ensures consistent behavior.

Log: Fixed wheel speed configuration validation to enforce 1-100 range

Influence:

  1. Test setting wheel speed to values below 1 (should clamp to 1)
  2. Test setting wheel speed to values above 100 (should clamp to 100)
  3. Test setting wheel speed to values within 1-100 range (should use exact value)
  4. Verify that imwheel process management still works correctly after validation
  5. Check that debug logging continues to function properly

fix: 在setWheelSpeed中添加滚轮速度值合法性校验

为滚轮速度配置值添加范围验证,确保其保持在1-100的有效范围内。之前代码仅
有注释说明有效范围但无实际验证。现在如果配置速度低于1将被设为1,高于100
将被设为100。这防止使用无效速度值并确保行为一致。

Log: 修复滚轮速度配置验证以强制执行1-100范围
PMS: BUG-348133
Influence:

  1. 测试将滚轮速度设置为低于1的值(应限制为1)
  2. 测试将滚轮速度设置为高于100的值(应限制为100)
  3. 测试将滚轮速度设置为1-100范围内的值(应使用精确值)
  4. 验证imwheel进程管理在验证后仍正常工作
  5. 检查调试日志记录是否继续正常运行

Summary by Sourcery

Bug Fixes:

  • Clamp configured wheel speed values to the valid range of 1–100 before applying them.

Added range validation for wheel speed configuration values to ensure
they stay within the valid range of 1-100. Previously, the code only had
a comment indicating the valid range but no actual validation. Now if
the configured speed is below 1, it will be set to 1, and if above 100,
it will be set to 100. This prevents invalid speed values from being
used and ensures consistent behavior.

Log: Fixed wheel speed configuration validation to enforce 1-100 range

Influence:
1. Test setting wheel speed to values below 1 (should clamp to 1)
2. Test setting wheel speed to values above 100 (should clamp to 100)
3. Test setting wheel speed to values within 1-100 range (should use
exact value)
4. Verify that imwheel process management still works correctly after
validation
5. Check that debug logging continues to function properly

fix: 在setWheelSpeed中添加滚轮速度值合法性校验

为滚轮速度配置值添加范围验证,确保其保持在1-100的有效范围内。之前代码仅
有注释说明有效范围但无实际验证。现在如果配置速度低于1将被设为1,高于100
将被设为100。这防止使用无效速度值并确保行为一致。

Log: 修复滚轮速度配置验证以强制执行1-100范围
PMS: BUG-348133
Influence:
1. 测试将滚轮速度设置为低于1的值(应限制为1)
2. 测试将滚轮速度设置为高于100的值(应限制为100)
3. 测试将滚轮速度设置为1-100范围内的值(应使用精确值)
4. 验证imwheel进程管理在验证后仍正常工作
5. 检查调试日志记录是否继续正常运行
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 26, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds runtime validation and clamping of the configured wheel speed to the documented valid range [1,100] before applying it, ensuring invalid configuration values are corrected instead of passed through.

Flow diagram for wheel speed validation in setWheelSpeed

flowchart TD
    A[setWheelSpeed called] --> B[Read speed from WheelSpeed.Get]
    B --> C{Is speed < 1?}
    C -- Yes --> D[Set speed = 1]
    C -- No --> E{Is speed > 100?}
    E -- Yes --> F[Set speed = 100]
    E -- No --> G[Keep original speed]
    D --> H[logger.Debug setWheelSpeed speed]
    F --> H
    G --> H
    H --> I[Proceed with imwheel process management]
Loading

File-Level Changes

Change Details Files
Clamp wheel speed configuration value into the valid [1,100] range before use.
  • Introduce local min and max bounds (1 and 100) for wheel speed validation.
  • Retrieve the configured wheel speed and compare it against the bounds.
  • If the configured speed is below the minimum, set it to the minimum; if above the maximum, set it to the maximum.
  • Log the (potentially clamped) wheel speed value as before, preserving existing debug behavior.
inputdevices1/manager.go

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

@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见

1. 语法与逻辑审查

  • 类型转换冗余:代码中多次将 minmax 转换为 uint32。由于 speed 的类型已经是 uint32,可以直接定义 minmaxuint32 类型,避免重复转换,提高代码可读性。
  • 逻辑正确性:目前的 if-else 逻辑是正确的,能够将速度限制在 [1, 100] 范围内。

2. 代码质量与风格

  • 硬编码1100 作为魔法数字直接出现在代码中。虽然注释说明了范围,但将其定义为常量(如 const MinSpeed = 1)会更规范,方便后续维护和修改。
  • 代码简洁性:可以使用标准库的 minmax 函数(Go 1.21+)或者简单的比较运算来简化边界检查逻辑,使其更紧凑。

3. 性能

  • 该段代码性能开销极小,对性能几乎无影响。

4. 安全性

  • 数据验证:增加边界检查是一个很好的做法,防止了 speed 为 0 或过大时可能导致的潜在错误(例如后续调用系统命令时参数异常)。

改进建议代码

以下是结合上述意见改进后的代码版本:

const (
    minWheelSpeed uint32 = 1
    maxWheelSpeed uint32 = 100
)

func (m *Manager) setWheelSpeed() {
    speed := m.WheelSpeed.Get()
    
    // Ensure speed is within the valid range [1, 100]
    if speed < minWheelSpeed {
        speed = minWheelSpeed
    } else if speed > maxWheelSpeed {
        speed = maxWheelSpeed
    }
    
    logger.Debug("setWheelSpeed", speed)

    // 为了避免imwheel对kwin影响,先杀死imwheel
    // ... (后续代码)
}

或者,如果使用 Go 1.21 或更高版本,可以使用 min/max 函数进一步简化:

import "cmp" // Go 1.21+

// ...

func (m *Manager) setWheelSpeed() {
    speed := m.WheelSpeed.Get()
    
    // Clamp speed to range [1, 100]
    speed = cmp.Or(cmp.Or(speed > maxWheelSpeed, maxWheelSpeed), speed)
    // 注意:cmp.Or 的逻辑是 "如果第一个参数为 true,返回第二个;否则返回第三个"
    // 上面这行代码逻辑略显晦涩,更推荐下面的写法:
    
    speed = max(minWheelSpeed, min(speed, maxWheelSpeed))

    logger.Debug("setWheelSpeed", speed)
    // ...
}

最推荐的写法(兼顾兼容性和可读性):

func (m *Manager) setWheelSpeed() {
    const (
        minSpeed = 1
        maxSpeed = 100
    )

    speed := m.WheelSpeed.Get()
    
    // speed range is [minSpeed, maxSpeed]
    if speed < minSpeed {
        speed = minSpeed
    } else if speed > maxSpeed {
        speed = maxSpeed
    }
    
    logger.Debug("setWheelSpeed", speed)

    // 为了避免imwheel对kwin影响,先杀死imwheel
    // ...
}

总结:主要改进点在于将魔法数字提取为常量,并优化了类型转换,使代码更健壮、易读。

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Since speed is already a uint32, consider defining min and max as const uint32 values (e.g., const minSpeed uint32 = 1) to avoid repeated casting and make the range intent clearer.
  • If wheel speed clamping is or may become needed in other places, consider extracting this range enforcement into a small helper function (e.g., clampWheelSpeed) to centralize the logic.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Since `speed` is already a `uint32`, consider defining `min` and `max` as `const` `uint32` values (e.g., `const minSpeed uint32 = 1`) to avoid repeated casting and make the range intent clearer.
- If wheel speed clamping is or may become needed in other places, consider extracting this range enforcement into a small helper function (e.g., `clampWheelSpeed`) to centralize the logic.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

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

Details Needs approval from an approver in each of these files:

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

@fly602 fly602 merged commit 98e80ce into linuxdeepin:master Jan 26, 2026
17 checks passed
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.

3 participants