Improvement possibilities

[info-cppsp]

This is not one problem, but instead should be a general discussion thread.
I would also just write things up as a TODO list or idea list.

1. Check memory management. / reduce mem consumption.
   (~30MB just to show a number seems to be a LOT.)

2. Double-check pointer use.
   Pointers are very powerful, but also very dangerous.
   search for g_strdup_printf, check free
   use autoptr?
   init all ptrs to NULL

3. Add more OOP.
   Many functions for handling sensors could be simplified that way.

4. Rethink and document software architecture.
   Kinda goes with 3.

• update readme

5. The relocatable schemas problem. - kinda done, abandoned
   See here:
   [applet] Gsettings relocatable schemas mess mate-panel#675

6. Clean up make warnings, notes.
   GtkStock deprecation in sensor-applet.c can't be removed without replacing old ui manager in mate-panel-applet.c with new GtkBuilder.

7. Fix tabs - done

8. Clean up includes

9. check plugins for deprecation, remove or update them

Ideas, solutions, more things are welcome.

[raveit65]

Reading temp values from disks behind a hardware-raid-controller with a command line.
Here is a example what i do to read a disk behind a 3Ware controller.

#!/bin/bash

smartctl -d 3ware,1 -A /dev/twa0 | grep Temperature_Celsius | awk '{print $10}'

So, an option in preferences gui where i can add a user sensor like this.

1. name of the sensor
2. icon
3. command.

Well commands for h-r-c are specific and they have to support by a background programme.
Maybe this isn't to complicated and it is possible?
We have already a command line applet where to see how those bits can work.

[info-cppsp]

The first thing that comes to mind is, how much fun it would be to set up a sensor just to do something very annoying every two seconds...
The second thing is that changing the sensor schema to include a 'command' key needs some consideration...
The label and Icon is already changeable. Maybe make it possible to add user icons.

Probably the best would be to add a new plugin.
In plugin init do nothing or just load data from gsettings and in sensors_applet_plugin_get_sensor_value() just run the command from gsettings.
It would need some things added to the prefs dialog as well.

[info-cppsp]

lukefromdc brought up indentation.

/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*-
 * vim: sts=0 sw=8 ts=8 tw=78 noexpandtab
*/

This is from the start of udisks-plugin.c
It seems to set the right indentation in gedit/pluma/xed. (with modeline plugin)
The first line is the emacs syntax, the second is the vim.

I thought we should use it in all files.
The 2 questions to vote on:

1. Tabs or spaces?
2. Width?
   IMHO 4 spaces

link

---

Also which C standard?

[raveit65]

Personal, i would only use spaces to avoid differences in editors.

[info-cppsp]

@monsta
clem_l on IRC said that I should ask you these questions:

1. which C standard do you use for mate-desktop projects?
2. how much do / don't you like OOP?
   I'm looking at a part of the mate-sensors-applet - the sensors - which could maybe benefit from OOP
   they are accessed from many places and it would be nice to have a definition in only one place
   or two, bc of the gschema as well
3. Is there a preferred way to implement OOP in mate-desktop? I mean should I use GObject or GVariant or make my own struct?
4. actually there is a sensor struct already, maybe if I add some function pointers to it. or would that be bad?
   some kind of GVariant would be nice, bc the sensor data is written to GSettings
   but that would than need a lot of conversion functions between regular types and GVariants

[info-cppsp]

MSA Documentation comment

MSA Sensors:

before PR #54 :
start:
sensors_applet_init
sensors_applet_plugins_load_all - load plugins
plugins -> load sensors
sensors_applet_add_sensor - checks if sensor is already loaded or if it is in gsettings, loads it from there

opening prefs dialog:
prefs_dialog_close - saves sensors

end:
destroy_cb calls prefs_dialog_close - saves sensors
destroy_cb - frees all sensors

after PR #54 :
start:
sensors_applet_init
(new) load sensors from gsettings, ordered
sensors_applet_plugins_load_all - load plugins
plugins -> load sensors
sensors_applet_add_sensor - checks if sensor is already loaded or if it is in gsettings, loads it from there

opening prefs dialog:
prefs_dialog_close - saves sensors

end:
destroy_cb calls prefs_dialog_close - saves sensors
destroy_cb - frees all sensors

possible improvement:
start:
sensors_applet_init
load sensors from gsettings, ordered, (new) skip not enabled
(new) if loaded, skip: sensors_applet_plugins_load_all - load plugins
(new) if loaded, skip: plugins -> load sensors
sensors_applet_add_sensor - checks if sensor is already loaded or if it is in gsettings, loads it from there

opening prefs dialog:
(new) sensors_applet_plugins_load_all - load plugins
(new) plugins -> load sensors
prefs_dialog_close - saves sensors

end:
destroy_cb calls prefs_dialog_close - saves sensors
destroy_cb - frees all sensors

Explanation: This way only used sensors would be stored in memory, reducing mem consumption.
Rationale: Sensors would be very likely set up only once and then used as set up for a long time.

[info-cppsp]

MSA Documentation comment

MSA Plugins:

before:
start:
sensors_applet_init
sensors_applet_plugins_load_all - load plugins
(Plugins are not stored, only pointers to their API functions.)

running:
active_sensor_update - looks up sensor update fuction, which has been saved by sensors_applet_plugins_load_all inside the sensors_applet->plugins g_hash_table and calls it on the sensor

end:
plugins hashtable is not freed. At least the string keys should be, but they are the plugin names, which are const and they are not copied, so no problem. The values are function pointers, so also no need to free them.
required_plugins g_hash_table keys are freed as they are copies of the plugin names. (g_hash_table does not copy on insert!) The values are written with GINT_TO_POINTER(TRUE). (Likely a trick to store a boolean in a g_hash_table.) It is inserted into from sensors_applet_add_sensor, so that the getvaluefunctions from the normal plugins g_hash_table are not removed if they are still in use.

possible improvement:
start:
sensors_applet_init
sensors_applet_plugins_load_all - load plugins
(new) (Plugins ARE stored, as MSAPlugin GObjects in the sensors_applet->plugins g_hash_table.)

running:
(new) Plugins that have no active sensors are removed.
(new) Prefs dialog box runs sensors_applet_plugins_load_all again, so that you can activate sensors.
(new) Prefs dialog box removes Plugins that have no active sensors on close.

end:
(new) plugins are freed by emptying the plugins g_hash_table

[info-cppsp]

msa includes
before: