PowerStatus: improved API, minor fixes.

* ioctl() does not return a status_t; fixed its usage.
* Do not overwrite the cached battery info with bogus data (this is
  quite strict, though).
* Improved DriverInterface API with a more "natural" argument order.
* Simplified some code.

src/apps/powerstatus/ACPIDriverInterface.cpp

@@ -1,15 +1,18 @@
 /*
- * Copyright 2009, Haiku, Inc. All Rights Reserved.
+ * Copyright 2009-2015, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
+ *		Axel Dörfler, [email protected]
  *		Clemens Zeidler, [email protected]
  */
 
 
 #include "ACPIDriverInterface.h"
 
+#include <errno.h>
 #include <stdio.h>
+#include <string.h>
 
 #include <Autolock.h>
 #include <Directory.h>
@@ -82,15 +85,19 @@ Battery::InitCheck()
 
 
 status_t
-Battery::ReadBatteryInfo()
+Battery::UpdateBatteryInfo()
 {
-	status_t status;
-	status = ioctl(fDriverHandler, GET_BATTERY_INFO, &fCachedAcpiInfo,
-		sizeof(acpi_battery_info));
+	acpi_battery_info info;
+	if (ioctl(fDriverHandler, GET_BATTERY_INFO, &info,
+			sizeof(acpi_battery_info)) != 0)
+		return errno;
 
-	if (status != B_OK)
-		return status;
+	if ((fExtendedBatteryInfo.last_full_charge > 0
+			&& info.capacity > fExtendedBatteryInfo.last_full_charge)
+		|| info.capacity < 0)
+		return B_BAD_DATA;
 
+	fCachedInfo = info;
 	return B_OK;
 }
 
@@ -98,15 +105,16 @@ Battery::ReadBatteryInfo()
 status_t
 Battery::GetBatteryInfoCached(battery_info* info)
 {
-	info->state = fCachedAcpiInfo.state;
-	info->current_rate = fCachedAcpiInfo.current_rate;
-	info->capacity = fCachedAcpiInfo.capacity;
+	info->state = fCachedInfo.state;
+	info->current_rate = fCachedInfo.current_rate;
+	info->capacity = fCachedInfo.capacity;
 	info->full_capacity = fExtendedBatteryInfo.last_full_charge;
-	fRateBuffer.AddRate(fCachedAcpiInfo.current_rate);
-	if (fCachedAcpiInfo.current_rate > 0 && fRateBuffer.GetMeanRate() != 0)
-		info->time_left = 3600 * fCachedAcpiInfo.capacity
+
+	fRateBuffer.AddRate(fCachedInfo.current_rate);
+	if (fCachedInfo.current_rate > 0 && fRateBuffer.GetMeanRate() != 0) {
+		info->time_left = 3600 * fCachedInfo.capacity
 			/ fRateBuffer.GetMeanRate();
-	else
+	} else
 		info->time_left = -1;
 
 	return B_OK;
@@ -116,11 +124,11 @@ Battery::GetBatteryInfoCached(battery_info* info)
 status_t
 Battery::GetExtendedBatteryInfo(acpi_extended_battery_info* info)
 {
-	status_t status;
-	status = ioctl(fDriverHandler, GET_EXTENDED_BATTERY_INFO, info,
-		sizeof(acpi_extended_battery_info));
+	if (ioctl(fDriverHandler, GET_EXTENDED_BATTERY_INFO, info,
+			sizeof(acpi_extended_battery_info)) != 0)
+		return errno;
 
-	return status;
+	return B_OK;
 }
 
 
@@ -128,30 +136,21 @@ void
 Battery::_Init()
 {
 	uint32 magicId = 0;
-	fInitStatus = ioctl(fDriverHandler, IDENTIFY_DEVICE, &magicId,
-		sizeof(uint32));
-	if (fInitStatus != B_OK)
+	if (ioctl(fDriverHandler, IDENTIFY_DEVICE, &magicId, sizeof(uint32)) != 0) {
+		fInitStatus = errno;
 		return;
-
-	fInitStatus = ioctl(fDriverHandler, GET_EXTENDED_BATTERY_INFO,
-		&fExtendedBatteryInfo, sizeof(acpi_extended_battery_info));
-	if (fInitStatus != B_OK)
-		return;
-
-	fInitStatus = ioctl(fDriverHandler, GET_BATTERY_INFO, &fCachedAcpiInfo,
-		sizeof(acpi_battery_info));
+	}
+	fInitStatus = GetExtendedBatteryInfo(&fExtendedBatteryInfo);
 	if (fInitStatus != B_OK)
 		return;
 
 	printf("ACPI driver found\n");
-
+	UpdateBatteryInfo();
 }
 
 
-ACPIDriverInterface::~ACPIDriverInterface()
-{
-	for (int i = 0; i < fDriverList.CountItems(); i++)
-		delete fDriverList.ItemAt(i);
+// #pragma mark - ACPIDriverInterface
+
 
 ACPIDriverInterface::ACPIDriverInterface()
 	:
@@ -160,42 +159,40 @@ ACPIDriverInterface::ACPIDriverInterface()
 }
 
 
-const char* kDriverDir = "/dev/power";
+ACPIDriverInterface::~ACPIDriverInterface()
+{
+	for (int i = 0; i < fDriverList.CountItems(); i++)
+		delete fDriverList.ItemAt(i);
+}
 
 
 status_t
 ACPIDriverInterface::Connect()
 {
-	printf("ACPI connect\n");
 	return _FindDrivers(kDriverDir);
 }
 
 
 status_t
-ACPIDriverInterface::GetBatteryInfo(battery_info* info, int32 index)
+ACPIDriverInterface::GetBatteryInfo(int32 index, battery_info* info)
 {
 	BAutolock autolock(fInterfaceLocker);
 	if (index < 0 || index >= fDriverList.CountItems())
 		return B_ERROR;
 
-	status_t status;
-	status = fDriverList.ItemAt(index)->GetBatteryInfoCached(info);
-	return status;
+	return fDriverList.ItemAt(index)->GetBatteryInfoCached(info);
 }
 
 
 status_t
-ACPIDriverInterface::GetExtendedBatteryInfo(acpi_extended_battery_info* info,
-	int32 index)
+ACPIDriverInterface::GetExtendedBatteryInfo(int32 index,
+	acpi_extended_battery_info* info)
 {
 	BAutolock autolock(fInterfaceLocker);
 	if (index < 0 || index >= fDriverList.CountItems())
 		return B_ERROR;
 
-	status_t status;
-	status = fDriverList.ItemAt(index)->GetExtendedBatteryInfo(info);
-
-	return status;
+	return fDriverList.ItemAt(index)->GetExtendedBatteryInfo(info);
 }
 
 
@@ -207,10 +204,10 @@ ACPIDriverInterface::GetBatteryCount()
 
 
 status_t
-ACPIDriverInterface::_ReadBatteryInfo()
+ACPIDriverInterface::_UpdateBatteryInfo()
 {
 	for (int i = 0; i < fDriverList.CountItems(); i++)
-		fDriverList.ItemAt(i)->ReadBatteryInfo();
+		fDriverList.ItemAt(i)->UpdateBatteryInfo();
 
 	return B_OK;
 }
@@ -223,7 +220,7 @@ ACPIDriverInterface::_WatchPowerStatus()
 		// every two seconds
 
 	while (atomic_get(&fIsWatching) > 0) {
-		_ReadBatteryInfo();
+		_UpdateBatteryInfo();
 		Broadcast(kMsgUpdate);
 		acquire_sem_etc(fWaitSem, 1, B_RELATIVE_TIMEOUT, kUpdateInterval);
 	}
@@ -245,21 +242,18 @@ ACPIDriverInterface::_FindDrivers(const char* dirpath)
 		if (entry.IsDirectory()) {
 			if (_FindDrivers(path.Path()) == B_OK)
 				return B_OK;
-		}
-		else {
+		} else {
 			int32 handler = open(path.Path(), O_RDWR);
 			if (handler >= 0) {
 				printf("try %s\n", path.Path());
 				Battery* battery = new Battery(handler);
-				if (battery->InitCheck() == B_OK) {
-					fDriverList.AddItem(battery);
+				if (battery->InitCheck() == B_OK
+					&& fDriverList.AddItem(battery)) {
 					status = B_OK;
-				}
-				else
+				} else
 					delete battery;
 			}
 		}
-
 	}
 	return status;
 }

src/apps/powerstatus/ACPIDriverInterface.h

@@ -39,7 +39,7 @@ public:
 	status_t					InitCheck();
 
 	// Read battery info and update the cache.
-	status_t 					ReadBatteryInfo();
+	status_t 					UpdateBatteryInfo();
 	status_t 					GetBatteryInfoCached(battery_info* info);
 	status_t 					GetExtendedBatteryInfo(
 									acpi_extended_battery_info* info);
@@ -54,7 +54,7 @@ private:
 	acpi_extended_battery_info	fExtendedBatteryInfo;
 
 	RateBuffer					fRateBuffer;
-	acpi_battery_info			fCachedAcpiInfo;
+	acpi_battery_info			fCachedInfo;
 };
 
 
@@ -63,23 +63,23 @@ public:
 								ACPIDriverInterface();
 	virtual						~ACPIDriverInterface();
 
-	virtual status_t		Connect();
-	virtual status_t 		GetBatteryInfo(battery_info* info, int32 index);
-	virtual status_t	 	GetExtendedBatteryInfo(
-								acpi_extended_battery_info* info, int32 index);
+	virtual status_t			Connect();
+	virtual status_t 			GetBatteryInfo(int32 index, battery_info* info);
+	virtual status_t	 		GetExtendedBatteryInfo(int32 index,
+									acpi_extended_battery_info* info);
 
-	virtual int32			GetBatteryCount();
+	virtual int32				GetBatteryCount();
 
 protected:
 	// Read the battery info from the hardware.
-	virtual status_t 		_ReadBatteryInfo();
+	virtual status_t 			_UpdateBatteryInfo();
 
-	virtual void			_WatchPowerStatus();
-	virtual status_t		_FindDrivers(const char* dirpath);
+	virtual void				_WatchPowerStatus();
+	virtual status_t			_FindDrivers(const char* dirpath);
 
-	BObjectList<Battery>	fDriverList;
-
-	BLocker					fInterfaceLocker;
+private:
+	BLocker						fInterfaceLocker;
+	BObjectList<Battery>		fDriverList;
 };
 
 #endif	// ACPI_DRIVER_INTERFACE_H

src/apps/powerstatus/APMDriverInterface.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009, Haiku, Inc. All Rights Reserved.
+ * Copyright 2009-2015, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -9,43 +9,23 @@
 
 #include "APMDriverInterface.h"
 
-#ifdef HAIKU_TARGET_PLATFORM_HAIKU
-#	include <arch/x86/apm_defs.h>
-#	include <generic_syscall_defs.h>
-#	include <syscalls.h>
-	// temporary, as long as there is no real power state API
-#endif
+#include <arch/x86/apm_defs.h>
+#include <generic_syscall_defs.h>
+#include <syscalls.h>
 
 
 const bigtime_t kUpdateInterval = 2000000;
 		// every two seconds
 
 
-#ifndef HAIKU_TARGET_PLATFORM_HAIKU
-// definitions for the APM driver available for BeOS
-enum {
-	APM_CONTROL = B_DEVICE_OP_CODES_END + 1,
-	APM_DUMP_POWER_STATUS,
-	APM_BIOS_CALL,
-	APM_SET_SAFETY
-};
-
-#define BIOS_APM_GET_POWER_STATUS 0x530a
-#endif
-
-
 APMDriverInterface::~APMDriverInterface()
 {
-#ifndef HAIKU_TARGET_PLATFORM_HAIKU
-	close(fDevice);
-#endif
 }
 
 
 status_t
 APMDriverInterface::Connect()
 {
-#ifdef HAIKU_TARGET_PLATFORM_HAIKU
 	uint32 version = 0;
 	status_t status = _kern_generic_syscall(APM_SYSCALLS, B_SYSCALL_INFO,
 		&version, sizeof(version));
@@ -56,28 +36,17 @@ APMDriverInterface::Connect()
 	}
 
 	return status;
-
-#else
-	fDevice = open("/dev/misc/apm", O_RDONLY);
-	if (fDevice < 0) {
-		return B_ERROR;
-	}
-
-	return B_OK;
-#endif
 }
 
 
 status_t
-APMDriverInterface::GetBatteryInfo(battery_info* info, int32 index)
+APMDriverInterface::GetBatteryInfo(int32 index, battery_info* info)
 {
 	if (index != 0)
 		return B_BAD_VALUE;
 
 	info->current_rate = -1;
 
-#ifdef HAIKU_TARGET_PLATFORM_HAIKU
-	// TODO: retrieve data from APM kernel interface
 	apm_battery_info apmInfo;
 	status_t status = _kern_generic_syscall(APM_SYSCALLS, APM_GET_BATTERY_INFO,
 		&apmInfo, sizeof(apm_battery_info));
@@ -89,37 +58,14 @@ APMDriverInterface::GetBatteryInfo(battery_info* info, int32 index)
 	}
 
 	return status;
-#else
-	if (fDevice < 0)
-		return B_ERROR;
-
-	uint16 regs[6] = {0, 0, 0, 0, 0, 0};
-	regs[0] = BIOS_APM_GET_POWER_STATUS;
-	regs[1] = 0x1;
-	if (ioctl(fDevice, APM_BIOS_CALL, regs) == 0) {
-		bool online = (regs[1] >> 8) != 0 && (regs[1] >> 8) != 2;
-		info->state = online ? BATTERY_CHARGING : BATTERY_DISCHARGING;
-		info->capacity = regs[2] & 255;
-		if (info->capacity > 100)
-			info->capacity = -1;
-		info->full_capacity = 100;
-		info->time_left = info->capacity >= 0 ? regs[3] : -1;
-		if (info->time_left > 0xffff)
-			info->time_left = -1;
-		else if (info->time_left & 0x8000)
-			info->time_left = (info->time_left & 0x7fff) * 60;
-	}
-
-	return B_OK;
-#endif
 }
 
 
 status_t
-APMDriverInterface::GetExtendedBatteryInfo(acpi_extended_battery_info* info,
-	int32 index)
+APMDriverInterface::GetExtendedBatteryInfo(int32 index,
+	acpi_extended_battery_info* info)
 {
-	return B_ERROR;
+	return B_NOT_SUPPORTED;
 }
 
 
@@ -138,4 +84,3 @@ APMDriverInterface::_WatchPowerStatus()
 		acquire_sem_etc(fWaitSem, 1, B_RELATIVE_TIMEOUT, kUpdateInterval);
 	}
 }
-

src/apps/powerstatus/APMDriverInterface.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009, Haiku, Inc. All Rights Reserved.
+ * Copyright 2009-2015, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -14,21 +14,17 @@
 
 class APMDriverInterface : public PowerStatusDriverInterface {
 public:
-	virtual				~APMDriverInterface();
+	virtual						~APMDriverInterface();
 
-	virtual status_t	Connect();
-	virtual status_t 	GetBatteryInfo(battery_info* info, int32 index);
-	virtual status_t	GetExtendedBatteryInfo(acpi_extended_battery_info* info,
-							int32 index);
-	virtual int32		GetBatteryCount();
+	virtual status_t			Connect();
+	virtual status_t 			GetBatteryInfo(int32 index, battery_info* info);
+	virtual status_t			GetExtendedBatteryInfo(int32 index,
+									acpi_extended_battery_info* info);
+	virtual int32				GetBatteryCount();
 
 protected:
-	virtual void		_WatchPowerStatus();
-
-private:
-#ifndef HAIKU_TARGET_PLATFORM_HAIKU
-	int					fDevice;
-#endif
+	virtual void				_WatchPowerStatus();
 };
 
+
 #endif	// APM_DRIVER_INTERFACE_H

src/apps/powerstatus/DriverInterface.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009, Haiku, Inc. All Rights Reserved.
+ * Copyright 2009-2015, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -34,47 +34,49 @@ struct battery_info {
 /*! Handle a list of watcher and broadcast a messages to them. */
 class Monitor {
 public:
-	virtual				~Monitor();
+	virtual						~Monitor();
 
-	virtual status_t	StartWatching(BHandler* target);
-	virtual status_t	StopWatching(BHandler* target);
+	virtual status_t			StartWatching(BHandler* target);
+	virtual status_t			StopWatching(BHandler* target);
 
-	virtual void		Broadcast(uint32 message);
+	virtual void				Broadcast(uint32 message);
 
 protected:
-	WatcherList			fWatcherList;
+	WatcherList					fWatcherList;
 };
 
 
 class PowerStatusDriverInterface : public Monitor, public BReferenceable {
 public:
-						PowerStatusDriverInterface();
-						~PowerStatusDriverInterface();
+								PowerStatusDriverInterface();
+								~PowerStatusDriverInterface();
 
-	virtual status_t	StartWatching(BHandler* target);
-	virtual status_t	StopWatching(BHandler* target);
-	virtual void		Broadcast(uint32 message);
+	virtual status_t			StartWatching(BHandler* target);
+	virtual status_t			StopWatching(BHandler* target);
+	virtual void				Broadcast(uint32 message);
 
-	virtual status_t	Connect() = 0;
-	virtual void		Disconnect();
+	virtual status_t			Connect() = 0;
+	virtual void				Disconnect();
 
-	virtual status_t 	GetBatteryInfo(battery_info* status, int32 index) = 0;
-	virtual status_t 	GetExtendedBatteryInfo(acpi_extended_battery_info* info,
-							int32 index) = 0;
+	virtual status_t 			GetBatteryInfo(int32 index,
+									battery_info* status) = 0;
+	virtual status_t 			GetExtendedBatteryInfo(int32 index,
+									acpi_extended_battery_info* info) = 0;
 
-	virtual int32		GetBatteryCount() = 0;
+	virtual int32				GetBatteryCount() = 0;
 
 protected:
-	virtual void		_WatchPowerStatus() = 0;
+	virtual void				_WatchPowerStatus() = 0;
 
-	int32				fIsWatching;
-	sem_id				fWaitSem;
+protected:
+	int32						fIsWatching;
+	sem_id						fWaitSem;
 
 private:
-	static int32		_ThreadWatchPowerFunction(void* data);
+	static int32				_ThreadWatchPowerFunction(void* data);
 
-	thread_id			fThread;
-	BLocker				fListLocker;
+	thread_id					fThread;
+	BLocker						fListLocker;
 };
 
 

src/apps/powerstatus/ExtendedInfoWindow.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009, Haiku, Inc. All Rights Reserved.
+ * Copyright 2009-2015, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -324,7 +324,7 @@ ExtPowerStatusView::Update(bool force)
 		return;
 
 	acpi_extended_battery_info extInfo;
-	fDriverInterface->GetExtendedBatteryInfo(&extInfo, fBatteryID);
+	fDriverInterface->GetExtendedBatteryInfo(fBatteryID, &extInfo);
 
 	fBatteryInfoView->Update(fBatteryInfo, extInfo);
 	fBatteryInfoView->Invalidate();
@@ -338,8 +338,8 @@ ExtendedInfoWindow::ExtendedInfoWindow(PowerStatusDriverInterface* interface)
 	:
 	BWindow(BRect(100, 150, 500, 500), B_TRANSLATE("Extended battery info"),
 		B_TITLED_WINDOW,
-		B_NOT_RESIZABLE | B_NOT_ZOOMABLE | B_AVOID_FRONT |
-		B_ASYNCHRONOUS_CONTROLS),
+		B_NOT_RESIZABLE | B_NOT_ZOOMABLE | B_AVOID_FRONT
+			| B_ASYNCHRONOUS_CONTROLS),
 	fDriverInterface(interface),
 	fSelectedView(NULL)
 {

src/apps/powerstatus/PowerStatusView.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright 2006-2014, Haiku, Inc. All Rights Reserved.
+ * Copyright 2006-2015, Haiku, Inc. All Rights Reserved.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
@@ -62,7 +62,8 @@ const int32 kLowBatteryPercentage = 15;
 
 PowerStatusView::PowerStatusView(PowerStatusDriverInterface* interface,
 	BRect frame, int32 resizingMode,  int batteryID, bool inDeskbar)
-	: BView(frame, kDeskbarItemName, resizingMode,
+	:
+	BView(frame, kDeskbarItemName, resizingMode,
 		B_WILL_DRAW | B_FULL_UPDATE_ON_RESIZE),
 	fDriverInterface(interface),
 	fBatteryID(batteryID),
@@ -464,11 +465,11 @@ void
 PowerStatusView::_GetBatteryInfo(battery_info* batteryInfo, int batteryID)
 {
 	if (batteryID >= 0) {
-		fDriverInterface->GetBatteryInfo(batteryInfo, batteryID);
+		fDriverInterface->GetBatteryInfo(batteryID, batteryInfo);
 	} else {
 		for (int i = 0; i < fDriverInterface->GetBatteryCount(); i++) {
 			battery_info info;
-			fDriverInterface->GetBatteryInfo(&info, i);
+			fDriverInterface->GetBatteryInfo(i, &info);
 
 			if (i == 0)
 				*batteryInfo = info;