* [PATCH] Add rfkill plugin
@ 2009-07-28 16:34 Bastien Nocera
2009-07-28 20:06 ` Marcel Holtmann
2009-07-29 15:13 ` Bastien Nocera
0 siblings, 2 replies; 15+ messages in thread
From: Bastien Nocera @ 2009-07-28 16:34 UTC (permalink / raw)
To: BlueZ development
[-- Attachment #1: Type: text/plain, Size: 213 bytes --]
The plugin allows us to restore the previous power state on
adapters when the killswitch on them has been unblocked.
Otherwise we end up with the adapter disabled when coming back from a
soft killswitch.
Cheers
[-- Attachment #2: 0001-Add-rfkill-plugin.patch --]
[-- Type: text/x-patch, Size: 7193 bytes --]
>From bf8b7c07542bd7acb4f9f98ba2165475f0ad9d65 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Tue, 28 Jul 2009 17:25:34 +0100
Subject: [PATCH] Add rfkill plugin
The plugin allows us to restore the previous power state on
adapters when the killswitch on them has been unblocked.
---
plugins/Makefile.am | 3 +
plugins/rfkill.c | 199 +++++++++++++++++++++++++++++++++++++++++++++++++++
src/adapter.c | 23 ++++--
src/adapter.h | 1 +
4 files changed, 219 insertions(+), 7 deletions(-)
create mode 100644 plugins/rfkill.c
diff --git a/plugins/Makefile.am b/plugins/Makefile.am
index 9d9f970..9ba8180 100644
--- a/plugins/Makefile.am
+++ b/plugins/Makefile.am
@@ -15,6 +15,9 @@ endif
builtin_modules += hciops
builtin_sources += hciops.c
+builtin_modules += rfkill
+builtin_sources += rfkill.c
+
if NETLINK
plugin_LTLIBRARIES += netlink.la
netlink_la_LIBADD = @NETLINK_LIBS@
diff --git a/plugins/rfkill.c b/plugins/rfkill.c
new file mode 100644
index 0000000..fad8b47
--- /dev/null
+++ b/plugins/rfkill.c
@@ -0,0 +1,199 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2009 Bastien Nocera <hadess@hadess.net>
+ * Copyright (C) 2007-2009 Intel Corporation. All rights reserved.
+ *
+ * Author:
+ * Bastien Nocera <bnocera@redhat.com>, based on code by
+ * Johannes Berg <johannes@sipsolutions.net>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+#include <glib.h>
+
+#include <dbus/dbus.h>
+#include <bluetooth/bluetooth.h>
+#include <bluetooth/sdp.h>
+
+#include "hcid.h"
+#include "plugin.h"
+#include "logging.h"
+
+#include "manager.h"
+#include "adapter.h"
+#include "device.h"
+
+enum rfkill_type {
+ RFKILL_TYPE_ALL = 0,
+ RFKILL_TYPE_WLAN,
+ RFKILL_TYPE_BLUETOOTH,
+ RFKILL_TYPE_UWB,
+ RFKILL_TYPE_WIMAX,
+ RFKILL_TYPE_WWAN,
+};
+
+enum rfkill_operation {
+ RFKILL_OP_ADD = 0,
+ RFKILL_OP_DEL,
+ RFKILL_OP_CHANGE,
+ RFKILL_OP_CHANGE_ALL,
+};
+
+struct rfkill_event {
+ uint32_t idx;
+ uint8_t type;
+ uint8_t op;
+ uint8_t soft;
+ uint8_t hard;
+};
+
+static char *get_name(__u32 idx)
+{
+ char *filename, *name, *pos;
+
+ filename = g_strdup_printf("/sys/class/rfkill/rfkill%u/name", idx);
+ if (g_file_get_contents(filename, &name, NULL, NULL) == FALSE) {
+ g_free(filename);
+ return NULL;
+ }
+
+ g_free(filename);
+
+ pos = strchr(name, '\n');
+ if (pos)
+ *pos = '\0';
+
+ return name;
+}
+
+static gboolean rfkill_event(GIOChannel *chan,
+ GIOCondition cond, gpointer data)
+{
+ unsigned char buf[32];
+ struct rfkill_event *event = (void *) buf;
+ char *sysname;
+ gboolean blocked;
+ gsize len;
+ GIOError err;
+
+ if (cond & (G_IO_NVAL | G_IO_HUP | G_IO_ERR))
+ return FALSE;
+
+ memset(buf, 0, sizeof(buf));
+
+ err = g_io_channel_read(chan, (gchar *) buf, sizeof(buf), &len);
+ if (err) {
+ if (err == G_IO_ERROR_AGAIN)
+ return TRUE;
+ return FALSE;
+ }
+
+ if (len != sizeof(struct rfkill_event))
+ return TRUE;
+
+ debug("idx %u type %u op %u soft %u hard %u",
+ event->idx, event->type, event->op,
+ event->soft, event->hard);
+
+ blocked = (event->soft || event->hard) ? TRUE : FALSE;
+ /* We already disable devices correctly when rfkilled */
+ if (blocked)
+ return TRUE;
+
+ sysname = get_name(event->idx);
+ if (sysname == NULL)
+ return TRUE;
+ if (g_str_has_prefix(sysname, "hci") == FALSE) {
+ debug("Ignoring unblocked killswitch '%s'", sysname);
+ g_free(sysname);
+ return TRUE;
+ }
+
+ switch (event->type) {
+ case RFKILL_TYPE_ALL:
+ case RFKILL_TYPE_BLUETOOTH: {
+ struct btd_adapter *adapter;
+ int id;
+
+ id = atoi(sysname + strlen("hci"));
+ adapter = manager_find_adapter_by_id(id);
+ if (adapter)
+ adapter_set_powered(adapter, TRUE);
+ break;
+ }
+ default:
+ break;
+ }
+
+ g_free(sysname);
+
+ return TRUE;
+}
+
+static GIOChannel *channel = NULL;
+
+static int rfkill_init(void)
+{
+ int fd;
+
+ debug("Init rfkill plugin");
+
+ if (main_opts.remember_powered == FALSE)
+ return 0;
+
+ fd = open("/dev/rfkill", O_RDWR);
+ if (fd < 0) {
+ debug("No rfkill support in the kernel");
+ return -EIO;
+ }
+
+ channel = g_io_channel_unix_new(fd);
+ g_io_channel_set_close_on_unref(channel, TRUE);
+
+ g_io_add_watch(channel, G_IO_IN | G_IO_NVAL | G_IO_HUP | G_IO_ERR,
+ rfkill_event, NULL);
+
+ return 0;
+}
+
+static void rfkill_exit(void)
+{
+ debug("Cleanup rfkill plugin");
+
+ if (channel == NULL)
+ return;
+
+ g_io_channel_shutdown(channel, TRUE, NULL);
+ g_io_channel_unref(channel);
+
+ channel = NULL;
+}
+
+BLUETOOTH_PLUGIN_DEFINE(rfkill, VERSION,
+ BLUETOOTH_PLUGIN_PRIORITY_DEFAULT, rfkill_init, rfkill_exit)
diff --git a/src/adapter.c b/src/adapter.c
index 06640e7..06c3018 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -483,21 +483,30 @@ done:
return 0;
}
-static DBusMessage *set_powered(DBusConnection *conn, DBusMessage *msg,
- gboolean powered, void *data)
+int adapter_set_powered(struct btd_adapter *adapter, gboolean powered)
{
- struct btd_adapter *adapter = data;
uint8_t mode;
- int err;
mode = powered ? get_mode(&adapter->bdaddr, "on") : MODE_OFF;
if (mode == adapter->mode)
- return dbus_message_new_method_return(msg);
+ return -EALREADY;
- err = set_mode(adapter, mode);
- if (err < 0)
+ return set_mode(adapter, mode);
+}
+
+static DBusMessage *set_powered(DBusConnection *conn, DBusMessage *msg,
+ gboolean powered, void *data)
+{
+ struct btd_adapter *adapter = data;
+ int err;
+
+ err = adapter_set_powered(adapter, powered);
+ if (err < 0) {
+ if (err == -EALREADY)
+ return dbus_message_new_method_return(msg);
return failed_strerror(msg, -err);
+ }
return dbus_message_new_method_return(msg);
}
diff --git a/src/adapter.h b/src/adapter.h
index d34fb80..fa02d5d 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -110,6 +110,7 @@ void adapter_get_address(struct btd_adapter *adapter, bdaddr_t *bdaddr);
void adapter_set_state(struct btd_adapter *adapter, int state);
int adapter_get_state(struct btd_adapter *adapter);
gboolean adapter_is_ready(struct btd_adapter *adapter);
+int adapter_set_powered(struct btd_adapter *adapter, gboolean powered);
struct remote_dev_info *adapter_search_found_devices(struct btd_adapter *adapter,
struct remote_dev_info *match);
void adapter_update_found_devices(struct btd_adapter *adapter, bdaddr_t *bdaddr,
--
1.6.2.5
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-28 16:34 [PATCH] Add rfkill plugin Bastien Nocera
@ 2009-07-28 20:06 ` Marcel Holtmann
2009-07-28 20:12 ` Bastien Nocera
2009-07-29 15:13 ` Bastien Nocera
1 sibling, 1 reply; 15+ messages in thread
From: Marcel Holtmann @ 2009-07-28 20:06 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
Hi Bastien,
> The plugin allows us to restore the previous power state on
> adapters when the killswitch on them has been unblocked.
>
> Otherwise we end up with the adapter disabled when coming back from a
> soft killswitch.
I think that having this as a plugin is overkill. I would just integrate
this as src/rfkill.c. Especially since it has no dependencies.
Regards
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-28 20:06 ` Marcel Holtmann
@ 2009-07-28 20:12 ` Bastien Nocera
2009-07-28 20:21 ` Marcel Holtmann
0 siblings, 1 reply; 15+ messages in thread
From: Bastien Nocera @ 2009-07-28 20:12 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ development
On Tue, 2009-07-28 at 22:06 +0200, Marcel Holtmann wrote:
> Hi Bastien,
>
> > The plugin allows us to restore the previous power state on
> > adapters when the killswitch on them has been unblocked.
> >
> > Otherwise we end up with the adapter disabled when coming back from a
> > soft killswitch.
>
> I think that having this as a plugin is overkill. I would just integrate
> this as src/rfkill.c. Especially since it has no dependencies.
It would mean it's harder to change its behaviour. Surely, it would make
more sense for this to be a plugin than for HCI operations to be.
Given that you don't use an object system with typing, it's probably one
or 2 function calls off. To me, it makes sense to have parts of the core
as plugins if it means that the core is leaner because of it.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-28 20:12 ` Bastien Nocera
@ 2009-07-28 20:21 ` Marcel Holtmann
0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2009-07-28 20:21 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
Hi Bastien,
> > > The plugin allows us to restore the previous power state on
> > > adapters when the killswitch on them has been unblocked.
> > >
> > > Otherwise we end up with the adapter disabled when coming back from a
> > > soft killswitch.
> >
> > I think that having this as a plugin is overkill. I would just integrate
> > this as src/rfkill.c. Especially since it has no dependencies.
>
> It would mean it's harder to change its behaviour. Surely, it would make
> more sense for this to be a plugin than for HCI operations to be.
the HCI operation are becoming/are a plugin, because we wanna switch to
netlink in the long term. However to do this, we need to separate them
first.
> Given that you don't use an object system with typing, it's probably one
> or 2 function calls off. To me, it makes sense to have parts of the core
> as plugins if it means that the core is leaner because of it.
Okay. Since we have built-in plugins, it makes no big difference.
Regards
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-28 16:34 [PATCH] Add rfkill plugin Bastien Nocera
2009-07-28 20:06 ` Marcel Holtmann
@ 2009-07-29 15:13 ` Bastien Nocera
2009-07-29 19:49 ` Marcel Holtmann
1 sibling, 1 reply; 15+ messages in thread
From: Bastien Nocera @ 2009-07-29 15:13 UTC (permalink / raw)
To: BlueZ development
On Tue, 2009-07-28 at 17:34 +0100, Bastien Nocera wrote:
> The plugin allows us to restore the previous power state on
> adapters when the killswitch on them has been unblocked.
>
> Otherwise we end up with the adapter disabled when coming back from a
> soft killswitch.
Just a note that __u32 causes failures with newer GCCs (not sure why),
using uint32_t instead fixes the problem.
I'll send another patch when this one gets committed.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-29 15:13 ` Bastien Nocera
@ 2009-07-29 19:49 ` Marcel Holtmann
2009-07-29 20:15 ` Bastien Nocera
0 siblings, 1 reply; 15+ messages in thread
From: Marcel Holtmann @ 2009-07-29 19:49 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
Hi Bastien,
> > The plugin allows us to restore the previous power state on
> > adapters when the killswitch on them has been unblocked.
> >
> > Otherwise we end up with the adapter disabled when coming back from a
> > soft killswitch.
>
> Just a note that __u32 causes failures with newer GCCs (not sure why),
> using uint32_t instead fixes the problem.
>
> I'll send another patch when this one gets committed.
I didn't commit this patch, because it should be part of bluetoothd and
not a plugin and as I mentioned before honor the RememberPowered setting
for system where other entities control that.
However I did push a RFKILL skeleton that that all the lifting except
bringing the adapter up. Hijacking the set_powered D-Bus command is the
wrong approach. We need a properly exported adapter_up() function for
this.
Regards
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-29 19:49 ` Marcel Holtmann
@ 2009-07-29 20:15 ` Bastien Nocera
2009-07-29 20:23 ` Marcel Holtmann
0 siblings, 1 reply; 15+ messages in thread
From: Bastien Nocera @ 2009-07-29 20:15 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ development
On Wed, 2009-07-29 at 21:49 +0200, Marcel Holtmann wrote:
> Hi Bastien,
>
> > > The plugin allows us to restore the previous power state on
> > > adapters when the killswitch on them has been unblocked.
> > >
> > > Otherwise we end up with the adapter disabled when coming back from a
> > > soft killswitch.
> >
> > Just a note that __u32 causes failures with newer GCCs (not sure why),
> > using uint32_t instead fixes the problem.
> >
> > I'll send another patch when this one gets committed.
>
> I didn't commit this patch, because it should be part of bluetoothd and
> not a plugin
You said you didn't mind:
http://thread.gmane.org/gmane.linux.bluez.kernel/2981/focus=2986
> Okay. Since we have built-in plugins, it makes no big difference.
> and as I mentioned before honor the RememberPowered setting
> for system where other entities control that.
That's what that does:
+ if (main_opts.remember_powered == FALSE)
+ return 0;
> However I did push a RFKILL skeleton that that all the lifting except
> bringing the adapter up.
Which is pretty much the same as the code I sent, and the code in
connman. What's the point of putting this in the core when it does the
exact same as the plugin save for a level of indirection?
> Hijacking the set_powered D-Bus command is the
> wrong approach. We need a properly exported adapter_up() function for
> this.
I believe I tried exporting adapter_up() but it didn't work.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-29 20:15 ` Bastien Nocera
@ 2009-07-29 20:23 ` Marcel Holtmann
2009-07-29 20:40 ` Bastien Nocera
2009-07-29 21:44 ` Johan Hedberg
0 siblings, 2 replies; 15+ messages in thread
From: Marcel Holtmann @ 2009-07-29 20:23 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
Hi Bastien,
> > > > The plugin allows us to restore the previous power state on
> > > > adapters when the killswitch on them has been unblocked.
> > > >
> > > > Otherwise we end up with the adapter disabled when coming back from a
> > > > soft killswitch.
> > >
> > > Just a note that __u32 causes failures with newer GCCs (not sure why),
> > > using uint32_t instead fixes the problem.
> > >
> > > I'll send another patch when this one gets committed.
> >
> > I didn't commit this patch, because it should be part of bluetoothd and
> > not a plugin
>
> You said you didn't mind:
> http://thread.gmane.org/gmane.linux.bluez.kernel/2981/focus=2986
> > Okay. Since we have built-in plugins, it makes no big difference.
>
> > and as I mentioned before honor the RememberPowered setting
> > for system where other entities control that.
>
> That's what that does:
> + if (main_opts.remember_powered == FALSE)
> + return 0;
>
> > However I did push a RFKILL skeleton that that all the lifting except
> > bringing the adapter up.
>
> Which is pretty much the same as the code I sent, and the code in
> connman. What's the point of putting this in the core when it does the
> exact same as the plugin save for a level of indirection?
I changed my mind, because of the main_opts. We don't wanna export the
options in the long term. So using src/rfkill.c seems more logical at
this point of time.
> > Hijacking the set_powered D-Bus command is the
> > wrong approach. We need a properly exported adapter_up() function for
> > this.
>
> I believe I tried exporting adapter_up() but it didn't work.
Johan, Luiz, any reason why this would not work. What needs to be done
to bring up the adapter. Besides calling the ioctl() directly which we
don't wanna do anymore.
Regards
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-29 20:23 ` Marcel Holtmann
@ 2009-07-29 20:40 ` Bastien Nocera
2009-07-29 21:44 ` Johan Hedberg
1 sibling, 0 replies; 15+ messages in thread
From: Bastien Nocera @ 2009-07-29 20:40 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ development
On Wed, 2009-07-29 at 22:23 +0200, Marcel Holtmann wrote:
> Hi Bastien,
>
> > > > > The plugin allows us to restore the previous power state on
> > > > > adapters when the killswitch on them has been unblocked.
> > > > >
> > > > > Otherwise we end up with the adapter disabled when coming back from a
> > > > > soft killswitch.
> > > >
> > > > Just a note that __u32 causes failures with newer GCCs (not sure why),
> > > > using uint32_t instead fixes the problem.
> > > >
> > > > I'll send another patch when this one gets committed.
> > >
> > > I didn't commit this patch, because it should be part of bluetoothd and
> > > not a plugin
> >
> > You said you didn't mind:
> > http://thread.gmane.org/gmane.linux.bluez.kernel/2981/focus=2986
> > > Okay. Since we have built-in plugins, it makes no big difference.
> >
> > > and as I mentioned before honor the RememberPowered setting
> > > for system where other entities control that.
> >
> > That's what that does:
> > + if (main_opts.remember_powered == FALSE)
> > + return 0;
> >
> > > However I did push a RFKILL skeleton that that all the lifting except
> > > bringing the adapter up.
> >
> > Which is pretty much the same as the code I sent, and the code in
> > connman. What's the point of putting this in the core when it does the
> > exact same as the plugin save for a level of indirection?
>
> I changed my mind, because of the main_opts. We don't wanna export the
> options in the long term. So using src/rfkill.c seems more logical at
> this point of time.
We could have exported some of the settings through manager_get_*(), for
example manager_get_remember_powered(), for use by plugins.
The point is moot now anyway.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-29 20:23 ` Marcel Holtmann
2009-07-29 20:40 ` Bastien Nocera
@ 2009-07-29 21:44 ` Johan Hedberg
2009-07-29 21:45 ` Bastien Nocera
` (2 more replies)
1 sibling, 3 replies; 15+ messages in thread
From: Johan Hedberg @ 2009-07-29 21:44 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Bastien Nocera, BlueZ development
Hi Marcel,
On Wed, Jul 29, 2009, Marcel Holtmann wrote:
> > > Hijacking the set_powered D-Bus command is the
> > > wrong approach. We need a properly exported adapter_up() function for
> > > this.
> >
> > I believe I tried exporting adapter_up() but it didn't work.
>
> Johan, Luiz, any reason why this would not work. What needs to be done
> to bring up the adapter. Besides calling the ioctl() directly which we
> don't wanna do anymore.
adapter_up() is more of a callback that's responsible for doing the
necessary initializations *after* adapter has just gone up, so it's not
the right function to call when you want to bring it up (i.e. call the
ioctl). I believe all code paths for bringing the adapter up call set_mode
in src/adapter.c which in turn calls adapter_ops->set_powered (which calls
the ioctl in the case of hciops).
So having a btd_adapter_set_powered exported to plugins (which is what
Bastien's patch seems to do) makes sense to me in this case. I might
actually need something similar for maemo in order to handle our offline
mode better (maemo specific plugin to catch the MCE offline mode signal
and then call btd_adapter_set_powered).
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-29 21:44 ` Johan Hedberg
@ 2009-07-29 21:45 ` Bastien Nocera
2009-07-30 2:10 ` Luiz Augusto von Dentz
2009-07-30 2:30 ` Marcel Holtmann
2 siblings, 0 replies; 15+ messages in thread
From: Bastien Nocera @ 2009-07-29 21:45 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Marcel Holtmann, BlueZ development
On Thu, 2009-07-30 at 00:44 +0300, Johan Hedberg wrote:
> Hi Marcel,
>
> On Wed, Jul 29, 2009, Marcel Holtmann wrote:
> > > > Hijacking the set_powered D-Bus command is the
> > > > wrong approach. We need a properly exported adapter_up() function for
> > > > this.
> > >
> > > I believe I tried exporting adapter_up() but it didn't work.
> >
> > Johan, Luiz, any reason why this would not work. What needs to be done
> > to bring up the adapter. Besides calling the ioctl() directly which we
> > don't wanna do anymore.
>
> adapter_up() is more of a callback that's responsible for doing the
> necessary initializations *after* adapter has just gone up, so it's not
> the right function to call when you want to bring it up (i.e. call the
> ioctl). I believe all code paths for bringing the adapter up call set_mode
> in src/adapter.c which in turn calls adapter_ops->set_powered (which calls
> the ioctl in the case of hciops).
>
> So having a btd_adapter_set_powered exported to plugins (which is what
> Bastien's patch seems to do) makes sense to me in this case. I might
> actually need something similar for maemo in order to handle our offline
> mode better (maemo specific plugin to catch the MCE offline mode signal
> and then call btd_adapter_set_powered).
Hence why it would probably be a good thing to have as a plugin :)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-29 21:44 ` Johan Hedberg
2009-07-29 21:45 ` Bastien Nocera
@ 2009-07-30 2:10 ` Luiz Augusto von Dentz
2009-07-30 2:27 ` Marcel Holtmann
2009-07-30 2:30 ` Marcel Holtmann
2 siblings, 1 reply; 15+ messages in thread
From: Luiz Augusto von Dentz @ 2009-07-30 2:10 UTC (permalink / raw)
To: Marcel Holtmann, Bastien Nocera, BlueZ development
Hi,
On Wed, Jul 29, 2009 at 6:44 PM, Johan Hedberg<johan.hedberg@gmail.com> wro=
te:
> adapter_up() is more of a callback that's responsible for doing the
> necessary initializations *after* adapter has just gone up, so it's not
> the right function to call when you want to bring it up (i.e. call the
> ioctl). I believe all code paths for bringing the adapter up call set_mod=
e
> in src/adapter.c which in turn calls adapter_ops->set_powered (which call=
s
> the ioctl in the case of hciops).
>
> So having a btd_adapter_set_powered exported to plugins (which is what
> Bastien's patch seems to do) makes sense to me in this case. I might
> actually need something similar for maemo in order to handle our offline
> mode better (maemo specific plugin to catch the MCE offline mode signal
> and then call btd_adapter_set_powered).
Yep, sounds good to me too, plugins can have references to adapters so
I guess this is perfectly fine. Also I don't think there is much to be
protected here since powered property is readwrite and can be changed
by any dbus client. This also makes me wonder what is the purpose of
rfkill when we can anyone can set powered directly?
--=20
Luiz Augusto von Dentz
Engenheiro de Computa=E7=E3o
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-30 2:10 ` Luiz Augusto von Dentz
@ 2009-07-30 2:27 ` Marcel Holtmann
0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2009-07-30 2:27 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Bastien Nocera, BlueZ development
Hi Luiz,
> > adapter_up() is more of a callback that's responsible for doing the
> > necessary initializations *after* adapter has just gone up, so it's not
> > the right function to call when you want to bring it up (i.e. call the
> > ioctl). I believe all code paths for bringing the adapter up call set_mode
> > in src/adapter.c which in turn calls adapter_ops->set_powered (which calls
> > the ioctl in the case of hciops).
> >
> > So having a btd_adapter_set_powered exported to plugins (which is what
> > Bastien's patch seems to do) makes sense to me in this case. I might
> > actually need something similar for maemo in order to handle our offline
> > mode better (maemo specific plugin to catch the MCE offline mode signal
> > and then call btd_adapter_set_powered).
>
> Yep, sounds good to me too, plugins can have references to adapters so
> I guess this is perfectly fine. Also I don't think there is much to be
> protected here since powered property is readwrite and can be changed
> by any dbus client. This also makes me wonder what is the purpose of
> rfkill when we can anyone can set powered directly?
if it is rfkilled, then hciconfig hci0 up will return -ERFKILL. We fixed
this for the 2.6.31 kernel. WiFi behaves the same now. So you can't just
go ahead and bring it up again, you do have to unblock it. And you can
only unblock softkilled devices. If it is hardkilled via a hardware
switch for example, there is nothing you can do to bring it back up
except flipping the hardware switch.
Regards
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-29 21:44 ` Johan Hedberg
2009-07-29 21:45 ` Bastien Nocera
2009-07-30 2:10 ` Luiz Augusto von Dentz
@ 2009-07-30 2:30 ` Marcel Holtmann
2009-07-30 10:23 ` Johan Hedberg
2 siblings, 1 reply; 15+ messages in thread
From: Marcel Holtmann @ 2009-07-30 2:30 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Bastien Nocera, BlueZ development
Hi Johan,
> > > > Hijacking the set_powered D-Bus command is the
> > > > wrong approach. We need a properly exported adapter_up() function for
> > > > this.
> > >
> > > I believe I tried exporting adapter_up() but it didn't work.
> >
> > Johan, Luiz, any reason why this would not work. What needs to be done
> > to bring up the adapter. Besides calling the ioctl() directly which we
> > don't wanna do anymore.
>
> adapter_up() is more of a callback that's responsible for doing the
> necessary initializations *after* adapter has just gone up, so it's not
> the right function to call when you want to bring it up (i.e. call the
> ioctl). I believe all code paths for bringing the adapter up call set_mode
> in src/adapter.c which in turn calls adapter_ops->set_powered (which calls
> the ioctl in the case of hciops).
>
> So having a btd_adapter_set_powered exported to plugins (which is what
> Bastien's patch seems to do) makes sense to me in this case. I might
> actually need something similar for maemo in order to handle our offline
> mode better (maemo specific plugin to catch the MCE offline mode signal
> and then call btd_adapter_set_powered).
then I leave this with you to export that function correctly. It should
be a separate patch anyway.
I would disagree with the MCE offline handling as a plugin. First off
all, why is it not integrated into RFKILL. Second off all, it could just
use RFKILL or RememberPowered=false and change Powered by itself. The
listening to offline signal is the wrong design.
Regards
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Add rfkill plugin
2009-07-30 2:30 ` Marcel Holtmann
@ 2009-07-30 10:23 ` Johan Hedberg
0 siblings, 0 replies; 15+ messages in thread
From: Johan Hedberg @ 2009-07-30 10:23 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Bastien Nocera, BlueZ development, David.Weinehall
Hi Marcel,
On Thu, Jul 30, 2009, Marcel Holtmann wrote:
> > So having a btd_adapter_set_powered exported to plugins (which is what
> > Bastien's patch seems to do) makes sense to me in this case. I might
> > actually need something similar for maemo in order to handle our offline
> > mode better (maemo specific plugin to catch the MCE offline mode signal
> > and then call btd_adapter_set_powered).
>
> then I leave this with you to export that function correctly. It should
> be a separate patch anyway.
Ok. Maybe Bastien can split this out from his original patch? Note that
the function should probably have the btd_ prefix since the idea has been
that only functions like that get exported to plugins (not sure if it's
actually enforced yet though).
> I would disagree with the MCE offline handling as a plugin. First off
> all, why is it not integrated into RFKILL. Second off all, it could just
> use RFKILL or RememberPowered=false and change Powered by itself. The
> listening to offline signal is the wrong design.
You're probably right, however this is what I have to live with in the
current maemo version. I've added David to CC so he can comment on the
possibility of replacing MCE offline/normal mode signals with rfkill in
the future.
Johan
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2009-07-30 10:23 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-28 16:34 [PATCH] Add rfkill plugin Bastien Nocera
2009-07-28 20:06 ` Marcel Holtmann
2009-07-28 20:12 ` Bastien Nocera
2009-07-28 20:21 ` Marcel Holtmann
2009-07-29 15:13 ` Bastien Nocera
2009-07-29 19:49 ` Marcel Holtmann
2009-07-29 20:15 ` Bastien Nocera
2009-07-29 20:23 ` Marcel Holtmann
2009-07-29 20:40 ` Bastien Nocera
2009-07-29 21:44 ` Johan Hedberg
2009-07-29 21:45 ` Bastien Nocera
2009-07-30 2:10 ` Luiz Augusto von Dentz
2009-07-30 2:27 ` Marcel Holtmann
2009-07-30 2:30 ` Marcel Holtmann
2009-07-30 10:23 ` Johan Hedberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox