-
Notifications
You must be signed in to change notification settings - Fork 28
feat: add charging protection status display #394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds conditional logic to the dock power plugin so that when the battery is in NOT_CHARGED state, the tooltip and applet tip distinguish between normal not-charging at 100% and charging protection being active below 100%. Sequence diagram for updated battery tooltip messages in NOT_CHARGED statesequenceDiagram
actor User
participant DockApplet
participant PowerPlugin
participant BatteryService
User->>DockApplet: Hover_over_battery_icon()
DockApplet->>PowerPlugin: refreshTipsData()
PowerPlugin->>BatteryService: getBatteryState()
BatteryService-->>PowerPlugin: batteryState = NOT_CHARGED
PowerPlugin->>BatteryService: getBatteryPercentage()
BatteryService-->>PowerPlugin: percentage
alt percentage < 100
PowerPlugin-->>DockApplet: tips = Capacity_%1,_charging_protection_active
PowerPlugin-->>DockApplet: appletTips = Charging_protection_active
else percentage == 100
PowerPlugin-->>DockApplet: tips = Capacity_%1,_not_charging
PowerPlugin-->>DockApplet: appletTips = Not_charging
end
DockApplet-->>User: Show_tooltip_and_applet_tip()
Flow diagram for conditional tooltip logic in NOT_CHARGED stateflowchart TD
A[batteryState == NOT_CHARGED] --> B{percentage < 100}
B -->|Yes| C[Set tips to Capacity_%1,_charging_protection_active]
C --> D[Set appletTips to Charging_protection_active]
B -->|No| E[Set tips to Capacity_%1,_not_charging]
E --> F[Set appletTips to Not_charging]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The new
charging protection activebranch is keyed only onbatteryState == NOT_CHARGED && percentage < 100, which may mislabel normal unplugged states as protection mode; consider wiring this to an explicit charging-protection flag or state instead of inferring solely from percentage. - With the new logic,
batteryState == NOT_CHARGED && percentage == 100now bypasses theFULLY_CHARGED || percentage == 100branch; verify this ordering matches the intended state model, or consider restructuring the conditions so that the 100% case is handled in a single place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `charging protection active` branch is keyed only on `batteryState == NOT_CHARGED && percentage < 100`, which may mislabel normal unplugged states as protection mode; consider wiring this to an explicit charging-protection flag or state instead of inferring solely from percentage.
- With the new logic, `batteryState == NOT_CHARGED && percentage == 100` now bypasses the `FULLY_CHARGED || percentage == 100` branch; verify this ordering matches the intended state model, or consider restructuring the conditions so that the 100% case is handled in a single place.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (m_chargingProtectThreshold < CHARGING_PROTECT_THRESHOLD_MIN || m_chargingProtectThreshold >= CHARGING_PROTECT_THRESHOLD_MAX) { | ||
| m_chargingProtectThreshold = CHARGING_PROTECT_THRESHOLD_MIN; | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个else 分支,是不是不需要?否则下面的refreshTipsData()执行不到了,
Add conditional display for charging protection status in battery tips When battery is not charging and percentage is below 100%, show "charging protection active" message When battery is not charging but percentage is 100%, maintain original "not charging" message This provides clearer indication when charging protection feature is active versus normal not-charging state Log: Added charging protection status indication in battery tips Influence: 1. Test with battery percentage below 100% and charging protection active 2. Test with battery at 100% and not charging 3. Verify tooltip and applet tip display correct messages for each scenario 4. Check message consistency between tooltip and applet display 5. Test with different battery percentages to ensure proper conditional logic feat: 添加充电保护状态显示 在电池提示信息中添加充电保护状态的条件显示 当电池未充电且电量低于100%时,显示"充电保护已激活"信息 当电池未充电但电量为100%时,保持原有的"未充电"显示 这提供了更清晰的充电保护功能激活状态与普通未充电状态的区分 Log: 在电池提示信息中新增充电保护状态指示 Influence: 1. 测试电池电量低于100%且充电保护激活时的显示 2. 测试电池电量为100%且未充电时的显示 3. 验证工具提示和应用提示在不同场景下显示正确的消息 4. 检查工具提示和应用提示之间消息显示的一致性 5. 测试不同电池电量百分比以确保条件逻辑正确
deepin pr auto review我来对这个 diff 进行详细的代码审查:
优点:
需要改进的地方:
#define CHARGING_PROTECT_THRESHOLD_MAX 100这里最大值定义为100,但实际配置允许的最大值是99,建议改为: #define CHARGING_PROTECT_THRESHOLD_MAX 99
if (m_chargingProtectThreshold < CHARGING_PROTECT_THRESHOLD_MIN || m_chargingProtectThreshold >= CHARGING_PROTECT_THRESHOLD_MAX) {
m_chargingProtectThreshold = CHARGING_PROTECT_THRESHOLD_MIN;
}建议添加日志记录,方便调试: if (m_chargingProtectThreshold < CHARGING_PROTECT_THRESHOLD_MIN || m_chargingProtectThreshold >= CHARGING_PROTECT_THRESHOLD_MAX) {
qWarning() << "Invalid charging protect threshold:" << m_chargingProtectThreshold << ", using default value";
m_chargingProtectThreshold = CHARGING_PROTECT_THRESHOLD_MIN;
}
bool ok;
int threshold = m_config->value("chargingProtectThreshold").toInt(&ok);
if (!ok) {
qWarning() << "Failed to parse charging protect threshold";
m_chargingProtectThreshold = CHARGING_PROTECT_THRESHOLD_MIN;
return;
}
bool PowerPlugin::isChargingProtectionActive(int percentage) const {
return percentage < CHARGING_PROTECT_THRESHOLD_MAX &&
percentage >= m_chargingProtectThreshold;
}
总结: |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, 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 |
Add conditional display for charging protection status in battery tips When battery is not charging and percentage is below 100%, show "charging protection active" message
When battery is not charging but percentage is 100%, maintain original "not charging" message
This provides clearer indication when charging protection feature is active versus normal not-charging state
Log: Added charging protection status indication in battery tips
Influence:
feat: 添加充电保护状态显示
在电池提示信息中添加充电保护状态的条件显示
当电池未充电且电量低于100%时,显示"充电保护已激活"信息
当电池未充电但电量为100%时,保持原有的"未充电"显示
这提供了更清晰的充电保护功能激活状态与普通未充电状态的区分
Log: 在电池提示信息中新增充电保护状态指示
Influence:
Summary by Sourcery
Clarify battery status tips for non-charging states.
New Features:
Enhancements: