fix: correct initial color temperature value when enabled#1079
Open
fly602 wants to merge 1 commit intolinuxdeepin:masterfrom
Open
fix: correct initial color temperature value when enabled#1079fly602 wants to merge 1 commit intolinuxdeepin:masterfrom
fly602 wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
The issue was that when color temperature was enabled, the initial value was incorrect due to inconsistent default temperature values across different modules. The constant defaultTemperature was removed and replaced with consistent usage of _dsDefaultTemperatureManual. The manager's ColorTemperatureManual property is now properly initialized by calling getDefaultTemperatureManual() method which sets it to the correct system default value. Previously, the defaultTemperature constant (6500) was used in some places while _dsDefaultTemperatureManual was used in others, causing inconsistency. The fix ensures that all modules use the same default value obtained from the system configuration. Log: Fixed incorrect initial color temperature value when enabling color temperature feature Influence: 1. Test enabling color temperature feature and verify the initial value is correct 2. Test switching between different color temperature modes (Auto, Manual, Custom) 3. Verify that manual color temperature adjustments work properly 4. Test system behavior after reboot with color temperature enabled 5. Verify consistency of color temperature values across different display configurations fix: 修复开启色温时初始值不正确的问题 问题在于启用色温功能时，由于不同模块间默认温度值不一致， 导致初始值不正确。移除了 defaultTemperature 常量，统一使用 _dsDefaultTemperatureManual。管理器的 ColorTemperatureManual 属性现在通 过调用 getDefaultTemperatureManual() 方法正确初始化为系统默认值。 之前，部分代码使用 defaultTemperature 常量（6500），而其他部分使用 _dsDefaultTemperatureManual，导致不一致。修复后确保所有模块使用从系统配 置获取的相同默认值。 Log: 修复开启色温功能时初始值不正确的问题 Influence: 1. 测试启用色温功能，验证初始值是否正确 2. 测试在不同色温模式（自动、手动、自定义）间切换 3. 验证手动调节色温功能正常工作 4. 测试重启系统后色温功能的保持情况 5. 验证不同显示配置下色温值的一致性 PMS: BUG-355459 Change-Id: If2bd2b716a9a73e79dfe5634757e3f757988a4bc
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAligns the default manual color temperature handling across display modules by removing the hard‑coded constant, consistently using the system-derived _dsDefaultTemperatureManual value, and initializing the manager’s ColorTemperatureManual from getDefaultTemperatureManual(). Sequence diagram for color temperature manual initialization flowsequenceDiagram
actor User
participant DisplayService
participant Manager
participant SystemConfig
User->>DisplayService: enableColorTemperature()
DisplayService->>Manager: newManager(service)
activate Manager
Manager->>SystemConfig: getDefaultTemperatureManual()
activate SystemConfig
SystemConfig-->>Manager: _dsDefaultTemperatureManual
deactivate SystemConfig
Manager->>Manager: set ColorTemperatureManual = _dsDefaultTemperatureManual
Manager-->>DisplayService: initialized Manager instance
deactivate Manager
User->>DisplayService: requestCurrentColorTemperature()
DisplayService->>Manager: getColorTemperatureValue()
Manager-->>DisplayService: temperature based on consistent _dsDefaultTemperatureManual
DisplayService-->>User: show current color temperature
Updated class diagram for color temperature manager and configclassDiagram
class Manager {
+int ColorTemperatureManual
+int ColorTemperatureMode
+getColorTemperatureValue() int
+getDefaultTemperatureManual()
}
class UserMonitorModeConfig {
+int ColorTemperatureMode
+int ColorTemperatureManual
+int ColorTemperatureModeOn
+fix()
+clone() UserMonitorModeConfig
}
class RedshiftRunner {
+getValue() int
}
class SystemConfig {
+int _dsDefaultTemperatureManual
+getDefaultTemperatureManual() int
}
Manager --> RedshiftRunner : uses
Manager --> SystemConfig : reads _dsDefaultTemperatureManual
UserMonitorModeConfig --> SystemConfig : uses _dsDefaultTemperatureManual for defaults
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Now that
m.ColorTemperatureManualis only initialized viagetDefaultTemperatureManual(), double-check that this method is always invoked before any consumer readsColorTemperatureManual(including during manager construction and config load) to avoid transient zero/incorrect values. - Directly using the global
_dsDefaultTemperatureManualinsideUserMonitorModeConfig.fix()andgetDefaultUserMonitorModeConfig()couples config logic to global state; consider passing the default value in or centralizing default resolution in one place to make initialization order and behavior easier to reason about.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that `m.ColorTemperatureManual` is only initialized via `getDefaultTemperatureManual()`, double-check that this method is always invoked before any consumer reads `ColorTemperatureManual` (including during manager construction and config load) to avoid transient zero/incorrect values.
- Directly using the global `_dsDefaultTemperatureManual` inside `UserMonitorModeConfig.fix()` and `getDefaultUserMonitorModeConfig()` couples config logic to global state; consider passing the default value in or centralizing default resolution in one place to make initialization order and behavior easier to reason about.
## Individual Comments
### Comment 1
<location path="display1/color_temp.go" line_range="398-400" />
<code_context>
value = int(manual)
}
return value
case ColorTemperatureModeCustom:
- value := defaultTemperature
+ value := defaultTemperatureManual
if m.customColorTempFlag {
value = int(manual)
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Mixing `defaultTemperatureManual` and `_dsDefaultTemperatureManual` may lead to inconsistent defaults
Here, the `ColorTemperatureModeCustom` branch still bases its value on `defaultTemperatureManual`, while the final fallback now uses `int(_dsDefaultTemperatureManual)`. Since `_dsDefaultTemperatureManual` is loaded dynamically in `getDefaultTemperatureManual`, this can yield different defaults depending on the path taken. Please standardize on one source (dynamic or constant) within `getColorTemperatureValue` so the manual temperature default is consistent across all branches.
Suggested implementation:
```golang
// 日落时，返回手动设置的色温(from v20)
if value != int(_dsDefaultTemperatureManual) {
```
```golang
case ColorTemperatureModeCustom:
value := int(_dsDefaultTemperatureManual)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
398
to
400
| case ColorTemperatureModeCustom: | ||
| value := defaultTemperature | ||
| value := defaultTemperatureManual | ||
| if m.customColorTempFlag { |
There was a problem hiding this comment.
suggestion (bug_risk): Mixing defaultTemperatureManual and _dsDefaultTemperatureManual may lead to inconsistent defaults
Here, the ColorTemperatureModeCustom branch still bases its value on defaultTemperatureManual, while the final fallback now uses int(_dsDefaultTemperatureManual). Since _dsDefaultTemperatureManual is loaded dynamically in getDefaultTemperatureManual, this can yield different defaults depending on the path taken. Please standardize on one source (dynamic or constant) within getColorTemperatureValue so the manual temperature default is consistent across all branches.
Suggested implementation:
// 日落时，返回手动设置的色温(from v20)
if value != int(_dsDefaultTemperatureManual) { case ColorTemperatureModeCustom:
value := int(_dsDefaultTemperatureManual)
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
2 participants
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.
The issue was that when color temperature was enabled, the initial value was incorrect due to inconsistent default temperature values across different modules. The constant defaultTemperature was removed and replaced with consistent usage of _dsDefaultTemperatureManual. The manager's ColorTemperatureManual property is now properly initialized by calling getDefaultTemperatureManual() method which sets it to the correct system default value.
Previously, the defaultTemperature constant (6500) was used in some places while _dsDefaultTemperatureManual was used in others, causing inconsistency. The fix ensures that all modules use the same default value obtained from the system configuration.
Log: Fixed incorrect initial color temperature value when enabling color temperature feature
Influence:
fix: 修复开启色温时初始值不正确的问题
问题在于启用色温功能时，由于不同模块间默认温度值不一致，
导致初始值不正确。移除了 defaultTemperature 常量，统一使用
_dsDefaultTemperatureManual。管理器的 ColorTemperatureManual 属性现在通 过调用 getDefaultTemperatureManual() 方法正确初始化为系统默认值。
之前，部分代码使用 defaultTemperature 常量（6500），而其他部分使用
_dsDefaultTemperatureManual，导致不一致。修复后确保所有模块使用从系统配
置获取的相同默认值。
Log: 修复开启色温功能时初始值不正确的问题
Influence:
PMS: BUG-355459
Change-Id: If2bd2b716a9a73e79dfe5634757e3f757988a4bc
Summary by Sourcery
Unify color temperature defaults to use the system-provided manual temperature value, ensuring consistent initialization and behavior when the color temperature feature is enabled.
Bug Fixes:
Enhancements:
Chores: