broulik added inline comments.
INLINE COMMENTS
> powerdevilupowerbackend.cpp:55
> , m_kbdMaxBrightness(0)
> - , m_lidIsPresent(false), m_lidIsClosed(false), m_onBattery(false)
> + , m_lidIsPresent(false), m_lidIsClosed(false), m_onBattery(false),
> m_isLedBrightnessControl(false)
> {
Put new var on a new line
> powerdevilupowerbackend.cpp:193
>
> - UdevQt::Client *client = new
> UdevQt::Client(QStringList("backlight"), this);
> - connect(client,
> SIGNAL(deviceChanged(UdevQt::Device)), SLOT(onDeviceChanged(UdevQt::Device)));
> + m_isLedBrightnessControl =
> m_syspath.split('/').contains("leds");
> + if (!m_isLedBrightnessControl) {
splitRef and QLatin1String
Can't you do contains("/leds/") to reduce the likelihood of false positives? Or
can it also be /sys/class/ledsfoo/ or sth like that?
> powerdevilupowerbackend.cpp:454
> + m_cachedBrightnessMap[Screen] = value;
> + slotScreenBrightnessChanged();
> + }
Make sure you don't emit if it didn't actually change (dunno what the code does
internally so might already check for that)
REPOSITORY
rPOWERDEVIL Powerdevil
REVISION DETAIL
https://phabricator.kde.org/D2470
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: bshah, #plasma, broulik
Cc: graesslin, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts,
sebas
