[energy kcm] Fix Technology values
ClosedPublic

Authored by meven on Oct 3 2019, 6:49 AM.

Details

Summary

Fix incorrect value of Technology enum.
Thanks @ltoscano for the catch.
Relates to D23392

Diff Detail

Repository
R102 KInfoCenter
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
meven created this revision.Oct 3 2019, 6:49 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 3 2019, 6:49 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
meven requested review of this revision.Oct 3 2019, 6:49 AM
meven updated this revision to Diff 67240.Oct 3 2019, 6:50 AM

Lower case technology

alexde added a subscriber: alexde.Oct 3 2019, 11:06 AM

As this it not in the scope of this patch, those values look suspicious to me too:

switch(model.battery.type) {
case 3: return i18n("Internal battery")
case 2: return i18n("UPS battery")
case 9: return i18n("Monitor battery")
case 4: return i18n("Mouse battery")
case 5: return i18n("Keyboard battery")
case 1: return i18n("PDA battery")
case 7: return i18n("Phone battery")
default: return i18n("Unknown battery")

Is there a special reason for the weired order? Is it correct that case 0 and 6 are missing and default to unknown? Maybe (I haven't looked deeper into it) this needs another patch.

Modules/energy/package/contents/ui/main.qml
104–110

Wouldn't it be better to define 0 as a default value?

110

Now you have two times the case 6

This is already available and translated in solid.

solid/src/solid/devices/backends/upower/upowerdevice.cpp
142: return tr("Lithium Ion", "battery technology");

Can't we expose that instead of duplicating things in QML. Especially with horrific magic numbers instead of enums.

meven updated this revision to Diff 67258.Oct 3 2019, 1:28 PM

Fix enum values

meven marked an inline comment as done.Oct 3 2019, 1:31 PM
meven updated this revision to Diff 67261.Oct 3 2019, 2:11 PM

Use enum values for chargeState and technology

meven added a comment.EditedOct 3 2019, 4:31 PM

This is already available and translated in solid.

solid/src/solid/devices/backends/upower/upowerdevice.cpp
142: return tr("Lithium Ion", "battery technology");

Being a private function , this is quite complicated to expose and in solid's backend.

Can't we expose that instead of duplicating things in QML. Especially with horrific magic numbers instead of enums.

I have updated the code to use the enum properly.

meven added inline comments.Oct 4 2019, 10:27 AM
Modules/energy/batterymodel.cpp
31–32
meven marked an inline comment as done.Oct 4 2019, 10:27 AM
meven marked an inline comment as done.
davidedmundson accepted this revision.Oct 4 2019, 10:34 AM

already a million times better

This revision is now accepted and ready to land.Oct 4 2019, 10:34 AM
This revision was automatically updated to reflect the committed changes.