ahiemstra added a comment.
You are right, sorry. I was confusing /sys with /proc, which does have a lot
of "put all of this information in a single file" stuff going on. It would
still be nice if there was a better way of getting this information, but that
is not related to this change. So then I mostly have style nitpicks as comment.
INLINE COMMENTS
> acpi.c:96
>
> - if ( ( d = opendir( "/proc/acpi/battery" ) ) == NULL ) {
> - AcpiBatteryNum = -1;
> - AcpiBatteryOk = 0;
> - return;
> - } else {
> - AcpiBatteryNum = 0;
> - AcpiBatteryOk = 1;
> - while ( ( de = readdir( d ) ) )
> - if ( ( strcmp( de->d_name, "." ) != 0 ) && ( strcmp( de->d_name, ".."
> ) != 0 ) ) {
> - strncpy( AcpiBatteryNames[ AcpiBatteryNum ], de->d_name, 8 );
> - snprintf( s, sizeof( s ), "acpi/battery/%d/batterycharge",
> AcpiBatteryNum );
> - registerMonitor( s, "integer", printAcpiBatFill,
> printAcpiBatFillInfo, sm );
> - snprintf( s, sizeof( s ), "acpi/battery/%d/batteryusage",
> AcpiBatteryNum );
> - registerMonitor( s, "integer", printAcpiBatUsage,
> printAcpiBatUsageInfo, sm);
> - AcpiBatteryCharge[ AcpiBatteryNum ] = 0;
> - AcpiBatteryNum++;
> - }
> + d = opendir("/sys/class/power_supply/");
> + if (d != NULL) {
Can you clean up the indentation of this function? It should use 4 spaces for
indentation.
> acpi.c:112
>
> -
> -int updateAcpiBattery( void )
> -{
> - int i, fd;
> - char s[ ACPIFILENAMELENGTHMAX ];
> - size_t n;
> - char AcpiBatInfoBuf[ ACPIBATTERYINFOBUFSIZE ];
> - char AcpiBatStateBuf[ ACPIBATTERYSTATEBUFSIZE ];
> - char *p;
> - int AcpiBatCapacity = 1;
> - int AcpiBatRemainingCapacity = 0;
> -
> - if ( AcpiBatteryNum <= 0 )
> - return -1;
> -
> - for ( i = 0; i < AcpiBatteryNum; i++ ) {
> - /* get total capacity */
> - snprintf( s, sizeof( s ), "/proc/acpi/battery/%s/info",
> AcpiBatteryNames[ i ] );
> - if ( ( fd = open( s, O_RDONLY ) ) < 0 ) {
> - print_error( "Cannot open file \'%s\'!\n"
> - "Load the battery ACPI kernel module or\n"
> - "compile it into your kernel.\n", s );
> - AcpiBatteryOk = 0;
> - return -1;
> - }
> - if ( ( n = read( fd, AcpiBatInfoBuf, ACPIBATTERYINFOBUFSIZE - 1 ) ) ==
> - ACPIBATTERYINFOBUFSIZE - 1 ) {
> - log_error( "Internal buffer too small to read \'%s\'", s );
> - close( fd );
> - AcpiBatteryOk = 0;
> - return -1;
> - }
> - close( fd );
> - p = AcpiBatInfoBuf;
> - if ( p && strstr(p, "ERROR: Unable to read battery") )
> - return 0; /* If we can't read the battery, reuse the last value
> */
> - while ( ( p!= NULL ) && ( sscanf( p, "last full capacity: %d ",
> - &AcpiBatCapacity ) != 1 ) ) {
> - p = strchr( p, '\n' );
> - if ( p )
> - p++;
> - }
> - /* get remaining capacity */
> - snprintf( s, sizeof( s ), "/proc/acpi/battery/%s/state",
> AcpiBatteryNames[ i ] );
> - if ( ( fd = open( s, O_RDONLY ) ) < 0 ) {
> - print_error( "Cannot open file \'%s\'!\n"
> - "Load the battery ACPI kernel module or\n"
> - "compile it into your kernel.\n", s );
> - AcpiBatteryOk = 0;
> - return -1;
> +void printSysBatteryCharge(const char *cmd) {
> + int zone = 0;
Braces on newline for function definitions please.
> acpi.c:151
> + int state = 0;
> + if (current > 0 && maximum > 0) {
> + state = current * 100 / maximum;
I don't see why current needs to be >0 ? What if I have an empty battery?
> acpi.c:170
>
> -void printAcpiBatUsage( const char* cmd)
> -{
> - int i;
> +void printSysBatteryRate(const char *cmd) {
> + int zone = 0;
Braces on newline for function definitions please.
> acpi.c:328
>
> -static int getSysFileValue(const char *group, int value, const char *file) {
> +static int getSysFileValue(const char *class, const char *group, int value,
> const char *file) {
> static int shownError = 0;
Using `class` as name here seems a bit dangerous to me, if this code ever gets
compiled with a C++ compiler it will trip over this name. Maybe use className
instead?
> acpi.c:332
> char input_buf[ 100 ];
> - snprintf(th_file, sizeof(th_file), "/sys/class/thermal/%s%d/%s",group,
> value, file);
> + snprintf(th_file, sizeof(th_file), "/sys/class/%s/%s%d/%s",class, group,
> value, file);
> int fd = open(th_file, O_RDONLY);
Please add a space between , and class
REPOSITORY
R106 KSysguard
REVISION DETAIL
https://phabricator.kde.org/D25018
To: jjorge, davidedmundson, #plasma, ahiemstra
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2,
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed,
jensreuterberg, abetts, sebas, apol, ahiemstra, mart
