D28333: Add a new daemon for stats monitoring

[Arjen Hiemstra]

ahiemstra added inline comments.

INLINE COMMENTS

> client.cpp:45
> +{
> +    for (auto sensor : m_subscribedSensors) {
> +        sensor->unsubscribe();

This should probably use `qAsConst`.

> ksysguarddaemon.cpp:76
> +        auto factory = qobject_cast<KPluginFactory*>(plugin);
> +        if (!factory) {
> +            return;

Should we do some error logging here? Might help future plugin development.

> ksysguarddaemon.cpp:93
> +        m_providers.append(provider);
> +        for (auto container : provider->containers()) {
> +            m_containers[container->id()] = container;

const auto containers = provider->containers();
  for(auto container : containers) {

> ksysguarddaemon.cpp:111
> +    SensorInfoMap sensors;
> +    for (auto c : m_containers) {
> +        for(auto o : c->objects()) {

`qAsConst(m_containers)`

> ksysguarddaemon.cpp:112
> +    for (auto c : m_containers) {
> +        for(auto o : c->objects()) {
> +            for (auto p : o->sensors()) {

Like above, objects() should be in a const variable. (Also for sensors on the line below).

> ksysguarddaemon.cpp:201
> +
> +    for (auto client: m_clients) {
> +        client->sendFrame();

`qAsConst(m_clients)`

> mart wrote in AggregateSensor.cpp:199
> if this timer is needed for compression, shouldn't be the same timer always restarted?
> if i call singleshot 10 times, iirc that slot will be invoked delayed 10 times

It's guarded with "m_dataChangeQueued" which only gets reset after the timer triggers.

> SensorContainer.cpp:64
> +    connect(object, &QObject::destroyed, this, [object, this]() {
> +        emit objectRemoved(object); //FIXME this is going to crash
> +    });

Uh, so shouldn't we fix this?

> SensorContainer.h:56
> +     */
> +    void addSubObject(SensorObject *object);
> +Q_SIGNALS:

Maybe move this to private with `friend class SensorObject`? Though I don't really see why this is internal only. Also probably should rename it to `addObject`?

REPOSITORY
  R106 KSysguard

REVISION DETAIL
  https://phabricator.kde.org/D28333

To: davidedmundson, #plasma
Cc: ahiemstra, ivan, mart, zzag, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200422/3815959b/attachment.html>