On Mon 21. Aug - 13:42:57, Frank Seidel wrote:
> Hi,
>
> for getting more HAL-support to kpowersave i've extended
> Holgers first suggested patch for kpowersave (supporting
> the new CPUFreq addon in HAL) to also do suspending
> via DBus calls to HAL.
Great, thanks. Finally we get some progress here...
>
> I consider this patch as very first (alpha-state) attempt in
> this direction eventhough it works ok for me, but I'd be very
> happy to get some feedback on it.
>
> Have fun :),
> Frank
> Index: src/dbusPowersave.h
> ===================================================================
> --- src/dbusPowersave.h (Revision 2409)
> +++ src/dbusPowersave.h (Arbeitskopie)
> @@ -94,6 +94,12 @@
> //! default destructor
> ~dbusPowersaveConnection();
>
> + //!send a message with one argument to HAL
> + static bool dbusSendHalMessage(char *method, int dbus_type, void
*arg);
> +
> + //!send a message with no/void argument to HAL
> + static bool dbusSendVoidHalMessage(char *method);
> +
Frank, you told me that you are working on some generic helper
functions. Does this have something to do with these dbus methods? I've
got some functions floating around which are more generic to send dbus
messages.
> //! to get information if KPowersave is connected to DBUS/powersave
> bool isConnected();
> //! to get information about rights to acced powersaved via DBUS
> Index: src/pdaemon.cpp
> ===================================================================
> --- src/pdaemon.cpp (Revision 2409)
> +++ src/pdaemon.cpp (Arbeitskopie)
> @@ -288,53 +288,43 @@
> // already set to wanted policy, ignore
> return 0;
>
> - int ret = 1;
> - int err_code;
> + bool result;
>
> switch( policy ) {
> case CPU_HIGH:
> - err_code = dbusSendSimpleMessage(ACTION_MESSAGE,
> - "CpufreqPerformance");
> + result = dbus_conn->dbusSendHalMessage("SetCPUFreqGovernor",
> + DBUS_TYPE_STRING,
> + (char*)"performance");
> break;
> case CPU_AUTO:
> - err_code = dbusSendSimpleMessage(ACTION_MESSAGE,
> - "CpufreqDynamic");
> + if (!(result =
dbus_conn->dbusSendHalMessage("SetCPUFreqGovernor",
>
+ DBUS_TYPE_STRING,
>
+ (char*)"ondeman")))
^
Missing 'd'. Don't know if that was already in in my initial patch.
> + result =
dbus_conn->dbusSendHalMessage("SetCPUFreqGovernor",
>
+ DBUS_TYPE_STRING,
>
+ (char*)"userspace");
> break;
> case CPU_LOW:
> - err_code = dbusSendSimpleMessage(ACTION_MESSAGE,
> - "CpufreqPowersave");
> + result = dbus_conn->dbusSendHalMessage("SetCPUFreqGovernor",
> + DBUS_TYPE_STRING,
> + (char*)"powersave");
> break;
> default:
> return -1;
> break;
> }
>
> - if ( err_code != REPLY_SUCCESS && err_code != REPLY_ALREADY_SET ) {
> + if ( !result ) {
> cpufreq_policy = CPU_UNSUPP;
> update_info_cpufreq_policy_changed = true;
> - ret = -1;
> + return -1;
> }
>
> - switch(err_code){
> - case REPLY_HW_NOT_SUPPORTED:
> - ret=-1;
> - break;
> - case REPLY_ALREADY_SET:
> - return 0;
> - break;
> - case REPLY_SUCCESS:
> - cpufreq_policy = policy;
> - update_info_cpufreq_policy_changed = true;
> - break;
> - case REPLY_NO_RIGHTS:
> - ret=-1;
> - break;
> - default:
> - break;
> - }
> + cpufreq_policy = policy;
> + update_info_cpufreq_policy_changed = true;
>
> emit generalDataChanged();
> - return ret;
> + return 1;
> }
>
> /*!
> @@ -502,33 +492,20 @@
> standby_allowed = new_value;
> }
>
> - ret = dbusSendMessageWithReply( REQUEST_MESSAGE,
> - &reply,
> - "CpufreqPolicy",
> - DBUS_TYPE_INVALID );
> - if ( ret == REPLY_SUCCESS ) {
> - char *value;
> - dbusGetMessageString( reply, &value, 0 );
> + if (!(ret = dbus_conn->dbusSendHalMessage( "SetCPUFreqGovernor",
> + DBUS_TYPE_STRING,
> + (char*)"ondemand")))
> + ret = dbus_conn->dbusSendHalMessage( "SetCPUFreqGovernor",
> + DBUS_TYPE_STRING,
> + (char*)"userspace");
> +
>
> - if ( !strcmp( value, "performance" ) ) {
> - new_value = CPU_HIGH;
> - }
> - else if ( !strcmp( value, "powersave" ) ) {
> - new_value = CPU_LOW;
> - }
> - else if ( !strcmp( value, "dynamic" ) ) {
> - new_value = CPU_AUTO;
> - }
> - else {
> - new_value = CPU_UNSUPP;
> - }
> -
> - if (new_value != cpufreq_policy){
> - // update cpufreq menu things
> - update_info_cpufreq_policy_changed = true;
> - cpufreq_policy = new_value;
> - }
> - dbus_message_unref( reply );
> + if ( ret ) {
> + // update cpufreq menu things
> + update_info_cpufreq_policy_changed = true;
> + cpufreq_policy = CPU_AUTO;
> + } else {
> + new_value = CPU_UNSUPP;
> }
>
> ret = dbusSendMessageWithReply( REQUEST_MESSAGE,
> Index: src/dbusPowersave.cpp
> ===================================================================
> --- src/dbusPowersave.cpp (Revision 2409)
> +++ src/dbusPowersave.cpp (Arbeitskopie)
> @@ -59,6 +59,99 @@
> }
>
> /*!
> + * Use this member to send a message with one argument to HAL
> + * \param method name of the HAL-method
> + * \param dbus_type int describing the argument type, e.g.
DBUS_TYPE_STRING
> + * \param arg pointer to the real argument
> + * \return boolean with result if sending-try was successfull
> + * \retval true if succeeded
> + * \retval false if failed
> + */
> +bool dbusPowersaveConnection::dbusSendHalMessage(char *method, int
dbus_type, void *arg) {
> + DBusMessage *message;
> + DBusError error;
> + DBusConnection *connection;
> + DBusMessage *reply;
> +
> + //check if we have a argument
> + if (arg == NULL)
> + return dbusSendVoidHalMessage(method);
> +
> + dbus_error_init(&error);
> +
> + connection = dbus_bus_get(DBUS_BUS_SYSTEM, &error);
> +
> + if (dbus_error_is_set(&error)) {
> + fprintf(stderr, "could not get dbus connection: %s\n",
error.message);
> + dbus_error_free(&error);
> + return false;
> + }
> +
> + message = dbus_message_new_method_call("org.freedesktop.Hal",
> + "/org/freedesktop/Hal/devices/computer",
>
+ "org.freedesktop.Hal.Device.SystemPowerManagement",
> + method);
> +
> + if (!dbus_message_append_args(message, dbus_type, &arg,
DBUS_TYPE_INVALID)) {
> + fprintf(stderr, "could not append arg to dbus message\n");
> + return false;
> + }
> +
> + reply = dbus_connection_send_with_reply_and_block(connection, message,
-1, &error);
> + if (dbus_error_is_set(&error)) {
> + fprintf(stderr, "could not send dbus message: %s\n",
error.message);
> + dbus_error_free(&error);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/*!
> + * Use this member to send a message with no arguments to HAL
> + * \param method name of the HAL-method
> + * \return boolean with result if sending-try was successfull
> + * \retval true if succeeded
> + * \retval false if failed
> + */
> +bool dbusPowersaveConnection::dbusSendVoidHalMessage(char *method) {
> + DBusMessage *message;
> + DBusError error;
> + DBusConnection *connection;
> + DBusMessage *reply;
> +
> + //check argument
> + if (method == NULL) {
> + fprintf(stderr, "%s: internal error: argument was NULL\n",
__func__);
> + return false;
> + }
> +
> + dbus_error_init(&error);
> +
> + connection = dbus_bus_get(DBUS_BUS_SYSTEM, &error);
> +
> + if (dbus_error_is_set(&error)) {
> + fprintf(stderr, "could not get dbus connection: %s\n",
error.message);
> + dbus_error_free(&error);
> + return false;
> + }
> +
> + message = dbus_message_new_method_call("org.freedesktop.Hal",
> + "/org/freedesktop/Hal/devices/computer",
>
+ "org.freedesktop.Hal.Device.SystemPowerManagement",
> + method);
> +
> + reply = dbus_connection_send_with_reply_and_block(connection, message,
-1, &error);
> + if (dbus_error_is_set(&error)) {
> + fprintf(stderr, "could not send dbus message: %s\n",
error.message);
> + dbus_error_free(&error);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/*!
> * Use this SLOT to emit a reviced messages to the pdaemon.
> * \param type enum with the type of the message
> * \param _string String with the message
> Index: src/kpowersave.cpp
> ===================================================================
> --- src/kpowersave.cpp (Revision 2409)
> +++ src/kpowersave.cpp (Arbeitskopie)
> @@ -645,32 +645,20 @@
> bool kpowersave::do_suspend2disk(){
> myDebug ("kpowersave::do_suspend2disk");
>
> - int res;
> - // sent to admin interface if we are run as root
> - if (getuid() == 0)
> - res = dbusSendSimpleMessage(ADMIN_MESSAGE,
> - "SuspendToDisk");
> - else
> - res = dbusSendSimpleMessage(ACTION_MESSAGE,
> - "SuspendToDisk");
> + bool res;
> +
> + //sent suspend to hal
> + res = dbusPowersaveConnection::dbusSendVoidHalMessage("Hibernate");
>
> suspendType = "suspend2disk";
> - switch (res) {
> - case REPLY_SUCCESS:
> - return true;
> - case REPLY_DISABLED:
> + if (!res) {
> KPassivePopup::message(i18n("WARNING"),
> - i18n("Suspend to disk disabled by
administrator."),
> + i18n("Suspend to disk was not
successful."),
> SmallIcon("messagebox_warning", 20),
this, i18n("Warning"), 15000);
> - this->contextMenu()->setItemEnabled(SUSPEND2DISK_MENU_ID,
false);
> - break;
> - default:
> - KPassivePopup::message(i18n("WARNING"),
> - i18n("The powersave daemon must be running
in the background for a suspend to disk."),
> - SmallIcon("messagebox_warning", 20), this,
i18n("Warning"), 15000);
> - break;
> + return false;
> }
> - return false;
> +
> + return true;
> }
>
> /*!
> @@ -683,32 +671,21 @@
> */
> bool kpowersave::do_suspend2ram(){
> myDebug ("kpowersave::do_suspend2ram");
> + int auto_wakeup = 0; //for now a dummy var; is to be seconds until
autoresume
>
> - int res;
> - if (getuid() == 0)
> - res = dbusSendSimpleMessage(ADMIN_MESSAGE,
> - "SuspendToRam");
> - else
> - res = dbusSendSimpleMessage(ACTION_MESSAGE,
> - "SuspendToRam");
> + bool res;
> + //send message to hal
> + res = dbusPowersaveConnection::dbusSendHalMessage("Suspend",
DBUS_TYPE_INT32, (int*)&auto_wakeup );
>
> suspendType = "suspend2ram";
> - switch (res) {
> - case REPLY_SUCCESS:
> - return true;
> - case REPLY_DISABLED:
> + if (!res) {
> KPassivePopup::message(i18n("WARNING"),
> - i18n("Suspend to RAM disabled by
administrator."),
> + i18n("Suspend to RAM didn't work."),
> SmallIcon("messagebox_warning", 20),
this, i18n("Warning"), 15000);
> - this->contextMenu()->setItemEnabled(SUSPEND2RAM_MENU_ID,
false);
> - break;
> - default:
> - KPassivePopup::message(i18n("WARNING"),
> - i18n("The powersave daemon must be running
in the background for a suspend to RAM."),
> - SmallIcon("messagebox_warning", 20), this,
i18n("Warning"), 15000);
> - break;
> + return false;
> }
> - return false;
> +
> + return true;
> }
>
> /*!
Can you please split up the patches into logical items next time? For
example CPUFreq one part, Suspend via Hal another one.
Another thing: To get something going in this area, can you please create
a branch like branches/kpowersave/kpowersave-ng (feel free to chose a more
apropriate name), so that we can work together on the new branch and can
start committing. The first commits haven't to be perfect, we can do
cleanups later.
Regards,
Holger
_______________________________________________
powersave-devel mailing list
[email protected]
http://forge.novell.com/mailman/listinfo/powersave-devel
