Skip to content

feat: add Tlp mode and short idle support to power management#161

Merged
xionglinlin merged 1 commit into
linuxdeepin:masterfrom
xionglinlin:master
Jun 4, 2026
Merged

feat: add Tlp mode and short idle support to power management#161
xionglinlin merged 1 commit into
linuxdeepin:masterfrom
xionglinlin:master

Conversation

@xionglinlin
Copy link
Copy Markdown
Contributor

Add TLP mode and short idle state properties and methods to the power management D-Bus interface. This enables new power management capabilities for controlling system power states more granularly.

Log: Added TlpMode property and SetTlpMode method for TLP power management, plus ShortIdleState property and SetShortIdleState method for short idle state control

Influence:

  1. Verify TlpMode property is exposed correctly and returns expected values
  2. Test SetTlpMode with valid and invalid mode strings
  3. Verify ShortIdleState property correctly reflects current state
  4. Test SetShortIdleState with both true and false values
  5. Verify no regression in existing power management functionalities
  6. Test mock implementations for both new methods and properties

feat: 为电源管理模块添加Tlp模式和短idle支持

为电源管理D-Bus接口添加TLP模式和短空闲状态属性和方法。这使得系统电源状态
控制更加精细化。

Log: 新增TlpMode属性和SetTlpMode方法用于TLP电源管理,以及ShortIdleState 属性和SetShortIdleState方法用于短空闲状态控制

Influence:

  1. 验证TlpMode属性是否正确暴露并返回预期值
  2. 使用有效和无效的模式字符串测试SetTlpMode方法
  3. 验证ShortIdleState属性是否正确反映当前状态
  4. 测试SetShortIdleState方法,包括true和false值
  5. 验证现有电源管理功能无回归
  6. 测试新方法和属性的Mock实现

PMS: TASK-389737
Change-Id: Iec37ad890c9fb830c0f08c6514f6471045b614e4

Add TLP mode and short idle state properties and methods to the
power management D-Bus interface. This enables new power management
capabilities for controlling system power states more granularly.

Log: Added TlpMode property and SetTlpMode method for TLP power
management, plus ShortIdleState property and SetShortIdleState method
for short idle state control

Influence:
1. Verify TlpMode property is exposed correctly and returns expected
values
2. Test SetTlpMode with valid and invalid mode strings
3. Verify ShortIdleState property correctly reflects current state
4. Test SetShortIdleState with both true and false values
5. Verify no regression in existing power management functionalities
6. Test mock implementations for both new methods and properties

feat: 为电源管理模块添加Tlp模式和短idle支持

为电源管理D-Bus接口添加TLP模式和短空闲状态属性和方法。这使得系统电源状态
控制更加精细化。

Log: 新增TlpMode属性和SetTlpMode方法用于TLP电源管理,以及ShortIdleState
属性和SetShortIdleState方法用于短空闲状态控制

Influence:
1. 验证TlpMode属性是否正确暴露并返回预期值
2. 使用有效和无效的模式字符串测试SetTlpMode方法
3. 验证ShortIdleState属性是否正确反映当前状态
4. 测试SetShortIdleState方法,包括true和false值
5. 验证现有电源管理功能无回归
6. 测试新方法和属性的Mock实现

PMS: TASK-389737
Change-Id: Iec37ad890c9fb830c0f08c6514f6471045b614e4
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的 Git Diff 内容。本次修改主要为 DDE 的电源管理 DBus 接口新增了 TlpModeShortIdleState 两个属性及对应的设置方法,并更新了由代码生成器自动生成的 Go 绑定代码和 Mock 代码。

以下是我从语法逻辑、代码质量、代码性能和代码安全四个维度提出的审查意见和改进建议:

1. 语法与逻辑

  • 版权年份逻辑错误 (重要)
    • 问题:在 auto.goauto_mock.go 中,版权年份从 2018 - 2023 修改为了 2018 - 2026。使用未来的年份(2026)作为版权截止年份在法律和逻辑上是不严谨的,通常版权声明中的年份应该是作品发布或最后修改的实际年份。
    • 建议:如果是自动生成代码,建议修改代码生成器模板,使用当前实际年份(如 2024),或者采用动态获取年份的方式。如果是手动修改,请改回正确的当前年份。
  • DBus 接口定义缺乏校验逻辑
    • 问题:在 Power.xml 中,TlpMode 的类型是 s (string),ShortIdleState 的类型是 b (bool)。DBus 接口本身不限制字符串的具体内容。如果后端实现没有对传入的 mode 进行严格校验,恶意或错误的客户端可以传入任意字符串(如 "abcdef"),可能导致后端逻辑异常或引发未定义行为。
    • 建议:确保后端 SetTlpMode 方法的实现中,对传入的 mode 参数进行严格的白名单校验(例如只允许 "performance", "balance", "powersave" 等)。

2. 代码质量

  • 命名规范与可读性
    • 问题TlpMode 作为一个公开的 DBus 属性和方法名,缩写 Tlp 的含义不够直观。对于不熟悉 Linux 笔记本电源管理工具(TLP)的开发者来说,这个命名缺乏自解释性。
    • 建议:如果 Tlp 确实是指代 TLP (Linux Advanced Power Management),建议在 XML 文件或生成代码的注释中补充说明;如果可能,考虑使用更通用的命名,如 AdvancedPowerMode 等(当然,如果是强依赖 TLP 组件,保留当前命名也是可以的,但注释是必须的)。
  • 自动生成代码的维护性
    • 问题auto.goauto_mock.go 文件头明确标注了 Code generated by "./generator ./system/org.deepin.dde.power1"; DO NOT EDIT.。本次 Diff 中修改了生成代码的版权年份,这意味着要么是手动修改了生成产物,要么是修改了生成器模板。
    • 建议:如果是手动修改了生成产物,这是不规范的做法,下次重新生成会被覆盖;如果是修改了模板,请确保提交记录中包含生成器模板的修改,以保证一致性。

3. 代码性能

  • 同步调用的无缓冲 Channel 阻塞风险
    • 问题:在 auto.go 的同步方法实现中:
      func (v *interfacePower) SetTlpMode(flags dbus.Flags, mode string) error {
          return (<-v.GoSetTlpMode(flags, make(chan *dbus.Call, 1), mode).Done).Err
      }
      这里使用了 make(chan *dbus.Call, 1) 创建了缓冲为 1 的 channel。虽然这符合 dbus-go 的常见同步调用模式,但如果 DBus 底层通信出现极端延迟或死锁,这里的 <- 阻塞等待会导致调用此方法的 Goroutine 永久挂起。
    • 建议:对于涉及系统底层硬件状态修改的 DBus 调用,建议在业务层调用时考虑增加超时控制机制(如 context.WithTimeout),避免因为 DBus 守护进程无响应而导致前端 UI 卡死。不过,由于这是自动生成的代码,此处只需保持与原方法(如 SetMode)一致即可,超时控制应由上层业务逻辑负责。

4. 代码安全

  • DBus 权限控制缺失
    • 问题:在 Power.xml 中,SetTlpModeSetShortIdleStatemethod,允许外部进程修改系统的电源状态和 TLP 模式。但 XML 中并没有看到对应的 <annotation> 标签来限制哪些用户/组可以调用这些方法。如果 DBus 服务端配置(如 systemd service 文件或 DBus .conf 文件)没有做严格限制,普通用户甚至非特权进程都可以随意修改电源模式,这可能带来安全风险。
    • 建议:检查并确保在 DBus 的策略配置文件中,对 org.deepin.dde.power1SetTlpModeSetShortIdleState 方法配置了合适的权限,例如只允许属于 wheel 组或具有特定 PolicyKit 权限的用户进行写入操作。可以在 XML 中加上注解,例如:
      <method name="SetTlpMode">
          <annotation name="org.freedesktop.DBus.Method.NoReply" value="false"/>
          <!-- 建议添加 PolicyKit 动作注解,确保安全 -->
          <arg name="mode" type="s" direction="in"></arg>
      </method>
  • Mock 代码中的 Panic 风险
    • 问题:在 auto_mock.go 中,如果类型断言失败,代码会直接 panic
      ret, ok := mockArgs.Get(0).(*dbus.Call)
      if !ok {
          panic(fmt.Sprintf("assert: arguments: 0 failed because object wasn't correct type: %v", mockArgs.Get(0)))
      }
      虽然这是测试代码,不直接影响生产环境,但 panic 会导致整个测试进程崩溃,掩盖其他可能存在的测试失败。
    • 建议:由于这是自动生成的 Mock 代码,通常不需要修改。但在编写实际的单测用例时,请确保 Mock 返回值的类型严格匹配,避免触发 panic。

总结

本次代码变更主要是接口和绑定代码的扩展,整体结构清晰,与原有代码风格保持了一致。最需要关注的是版权年份的逻辑错误以及后端实现中对 TlpMode 字符串参数的校验,建议在合并前予以确认和修复。同时,请确保 DBus 服务端的权限配置跟上了新接口的开放。

Copy link
Copy Markdown
Contributor

@mhduiy mhduiy left a comment

Choose a reason for hiding this comment

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

Code Review: PR #161

整体来看,XML 定义和自动生成的代码(auto.goauto_mock.go)符合项目规范,代码结构清晰。有以下几个建议:

建议

  1. TlpMode 属性仅 access="read",是否需要 readwrite

    • XML 中 TlpMode 定义为 type="s" access="read",但从 dde-daemon 的实现来看,SetTlpMode 方法会间接改变 TLP 的实际配置状态。如果 TlpMode 属性始终是 read-only,调用方如何通过 property 变化信号感知 TLP 模式已切换?建议确认属性是否应该是 readwrite,或者是否有对应的 PropertiesChanged 信号机制。
  2. ShortIdleState 同理 - 当前定义为 access="read",session 侧通过 SetShortIdleState 修改状态后,是否有机制通知 property 变化?

  3. headRefName 为 master - 建议使用 feature 分支提交 PR,避免直接向 master 提交。

细节

  • ✅ 自动生成代码风格与现有代码一致
  • ✅ Mock 实现完整
  • ✅ Copyright 年份更新合理

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@xionglinlin xionglinlin merged commit 7a5cfd9 into linuxdeepin:master Jun 4, 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.

4 participants