Skip to content

fix: 当设置滚轮配置超出范围时,矫正配置#1035

Merged
fly602 merged 1 commit intolinuxdeepin:masterfrom
fly602:master
Feb 27, 2026
Merged

fix: 当设置滚轮配置超出范围时,矫正配置#1035
fly602 merged 1 commit intolinuxdeepin:masterfrom
fly602:master

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Feb 26, 2026

当设置滚轮配置超出范围时,矫正配置

Log: 当设置滚轮配置超出范围时,矫正配置
PMS: BUG-348133
Influence: 滚轮速度

Summary by Sourcery

Bug Fixes:

  • Clamp wheel speed to the valid range and update the stored wheel speed value when the configured speed is below the minimum or above the maximum.

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 26, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Ensures wheel speed settings are corrected to the valid range and applied when the configured value is out of bounds.

Sequence diagram for corrected wheel speed setting in setWheelSpeed

sequenceDiagram
    participant Manager
    participant WheelSpeed
    participant Logger

    Manager->>Manager: setWheelSpeed()
    Manager->>Manager: read configuredSpeed
    Manager->>Manager: determine min, max
    alt speed < min
        Manager->>Manager: speed = min
        Manager-->>WheelSpeed: defer Set(speed)
    else speed > max
        Manager->>Manager: speed = max
        Manager-->>WheelSpeed: defer Set(speed)
    else within range
        Note over Manager: no deferred Set
    end
    Manager-->>Logger: Debug("setWheelSpeed", speed)
    Manager-->>WheelSpeed: (deferred) Set(speed) when out of range
    Manager-->>Manager: return
Loading

File-Level Changes

Change Details Files
Clamp out-of-range wheel speed values and ensure the corrected value is applied to the device state.
  • When wheel speed is below the minimum, clamp it to the minimum and schedule updating WheelSpeed with the corrected value via defer
  • When wheel speed is above the maximum, clamp it to the maximum and schedule updating WheelSpeed with the corrected value via defer
  • Leave in-range wheel speeds unchanged while still logging the final value
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

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 found 1 issue, and left some high level feedback:

  • Using defer m.WheelSpeed.Set(speed) inside each bounds branch means the debug log will print the unclamped value and the setter will run later, which is likely unintended; consider calling Set immediately (or once after clamping) so the log and state reflect the corrected value synchronously.
  • If this method can be called frequently (e.g., in a loop or on high‑frequency events), deferring WheelSpeed.Set on each invocation may cause unnecessary stacking of deferred calls; it would be leaner and clearer to call Set directly after computing the final speed.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Using `defer m.WheelSpeed.Set(speed)` inside each bounds branch means the debug log will print the unclamped value and the setter will run later, which is likely unintended; consider calling `Set` immediately (or once after clamping) so the log and state reflect the corrected value synchronously.
- If this method can be called frequently (e.g., in a loop or on high‑frequency events), deferring `WheelSpeed.Set` on each invocation may cause unnecessary stacking of deferred calls; it would be leaner and clearer to call `Set` directly after computing the final `speed`.

## Individual Comments

### Comment 1
<location path="inputdevices1/manager.go" line_range="117" />
<code_context>
 	// speed range is [1,100]
 	if speed < uint32(min) {
 		speed = uint32(min)
+		defer m.WheelSpeed.Set(speed)
 	} else if speed > uint32(max) {
 		speed = uint32(max)
</code_context>
<issue_to_address>
**suggestion:** Using `defer` inside these branches is likely unnecessary and may be misleading or inefficient.

Since the only work after clamping is a debug log before returning, deferring `WheelSpeed.Set(speed)` adds indirection without real benefit and hides when the final value is applied. It also causes defers to accumulate on each call (e.g., in a loop) until return. Prefer calling `m.WheelSpeed.Set(speed)` immediately after clamping, or once at the end of the function, instead of deferring it inside the branches.
</issue_to_address>

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.

@fly602 fly602 force-pushed the master branch 2 times, most recently from 8939d25 to afa7061 Compare February 26, 2026 03:28
mhduiy
mhduiy previously approved these changes Feb 26, 2026
当设置滚轮配置超出范围时,矫正配置

Log: 当设置滚轮配置超出范围时,矫正配置
PMS: BUG-348133
Influence: 滚轮速度
@deepin-ci-robot
Copy link

deepin pr auto review

这份代码变更主要包含两部分内容:版权年份更新和逻辑调整。以下是对这两部分的详细审查和改进意见:

1. 版权年份变更

-// SPDX-FileCopyrightText: 2018 - 2022 UnionTech Software Technology Co., Ltd.
+// SPDX-FileCopyrightText: 2018 - 2026 UnionTech Software Technology Co., Ltd.

审查意见:

  • 逻辑与语法:语法正确,符合 SPDX 标识符规范。
  • 建议:通常版权年份应更新到当前年份(如 2023 或 2024),直接延长到 2026 年属于"预判"未来,虽然不报错,但在合规性上略显不严谨。建议改为当前年份。

2. setWheelSpeed 逻辑变更

 func (m *Manager) setWheelSpeed() {
 	// speed range is [1,100]
 	if speed < uint32(min) {
 		speed = uint32(min)
+		defer m.WheelSpeed.Set(speed)
 	} else if speed > uint32(max) {
 		speed = uint32(max)
+		defer m.WheelSpeed.Set(speed)
 	}
 	logger.Debug("setWheelSpeed", speed)

审查意见:

A. 逻辑与功能

  • 问题:原代码中,如果 speed 超出 [min, max] 范围,仅修正了局部变量 speed 的值,但似乎缺少了将修正后的值实际写入系统(调用 Set 方法)的逻辑。新增的 defer 补上了这一步,逻辑上意图是正确的。
  • 潜在 Bugdefer 语句会在函数退出时执行。
    • 如果 speed 本身就在合法范围内(既不 < min 也不 > max),代码中的两个 defer不会被执行。这意味着合法的速度值可能不会被 Set 进去(除非函数末尾有其他未展示的 Set 逻辑)。
    • 结论:如果目的是"无论速度是否越界,都要设置最终确定的 speed 值",那么目前的逻辑是有缺陷的,因为它只处理了越界情况。

B. 代码质量

  • 代码重复defer m.WheelSpeed.Set(speed) 出现了两次,违反了 DRY (Don't Repeat Yourself) 原则。
  • 可读性:在 if/else 分支内部使用 defer 会让控制流变得不直观。阅读代码时容易忽略它是在函数结束时才执行的,而不是立即执行。

C. 代码性能

  • defer 在 Go 中会有轻微的运行时开销(涉及函数调用和 defer 链维护)。虽然在这个场景下影响微乎其微,但为了设置一个值而引入 defer 并非必要,直接调用即可。

D. 代码安全

  • 并发安全:假设 m.WheelSpeed 是一个并发安全的属性(如 dbus 的 Property),则此处是安全的。如果 m.WheelSpeed 是普通的 struct 字段且未加锁保护,在并发环境下可能会有竞态条件。

改进建议代码

建议将设置逻辑统一处理,避免使用 defer,并确保无论是否越界都会执行设置操作。

func (m *Manager) setWheelSpeed() {
	// speed range is [1,100]
	// 假设 speed 是局部变量,且 min, max 已定义
	var speed uint32 = ... // 获取当前 speed 的逻辑

	// 修正 speed 值
	if speed < uint32(min) {
		speed = uint32(min)
	} else if speed > uint32(max) {
		speed = uint32(max)
	}

	// 统一在逻辑修正后进行设置,确保合法值也会被更新
	m.WheelSpeed.Set(speed)
	
	logger.Debug("setWheelSpeed", speed)
}

改进点总结:

  1. 修复逻辑漏洞:移除了 defer,改为直接调用,并确保无论是否进入 if 分支,最终的 speed 都会被 Set
  2. 消除重复代码:去除了重复的 defer 语句。
  3. 提升可读性:逻辑更加线性,先修正值,再应用值,一目了然。
  4. 微小性能提升:避免了 defer 的开销。

@fly602 fly602 merged commit a82ad8a into linuxdeepin:master Feb 27, 2026
16 of 17 checks passed
@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

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