D25018: Move ACPI battery information from /proc/acpi to /sys

[Arjen Hiemstra]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20191106/0f79ddb8/attachment-0001.html>