2014-02-14 15:52:57

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 1/8] obexd/transfer: Add Transfer.Suspend method

From: Luiz Augusto von Dentz <[email protected]>

This method can be used to suspend ongoing transfers.
---
obexd/client/transfer.c | 60 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 47 insertions(+), 13 deletions(-)

diff --git a/obexd/client/transfer.c b/obexd/client/transfer.c
index 5a8d4f2..813d20b 100644
--- a/obexd/client/transfer.c
+++ b/obexd/client/transfer.c
@@ -60,6 +60,7 @@ struct transfer_callback {
enum {
TRANSFER_STATUS_QUEUED = 0,
TRANSFER_STATUS_ACTIVE,
+ TRANSFER_STATUS_SUSPENDED,
TRANSFER_STATUS_COMPLETE,
TRANSFER_STATUS_ERROR
};
@@ -187,6 +188,41 @@ static DBusMessage *obc_transfer_cancel(DBusConnection *connection,
return NULL;
}

+static void transfer_set_status(struct obc_transfer *transfer, uint8_t status)
+{
+ if (transfer->status == status)
+ return;
+
+ transfer->status = status;
+
+ g_dbus_emit_property_changed(transfer->conn, transfer->path,
+ TRANSFER_INTERFACE, "Status");
+}
+
+static DBusMessage *obc_transfer_suspend(DBusConnection *connection,
+ DBusMessage *message, void *user_data)
+{
+ struct obc_transfer *transfer = user_data;
+ const char *sender;
+
+ sender = dbus_message_get_sender(message);
+ if (g_strcmp0(transfer->owner, sender) != 0)
+ return g_dbus_create_error(message,
+ ERROR_INTERFACE ".NotAuthorized",
+ "Not Authorized");
+
+ if (transfer->xfer == 0)
+ return g_dbus_create_error(message,
+ ERROR_INTERFACE ".NotInProgress",
+ "Not in progress");
+
+ g_obex_suspend(transfer->obex);
+
+ transfer_set_status(transfer, TRANSFER_STATUS_SUSPENDED);
+
+ return g_dbus_create_reply(message, DBUS_TYPE_INVALID);
+}
+
static gboolean name_exists(const GDBusPropertyTable *property, void *data)
{
struct obc_transfer *transfer = data;
@@ -269,6 +305,8 @@ static const char *status2str(uint8_t status)
return "queued";
case TRANSFER_STATUS_ACTIVE:
return "active";
+ case TRANSFER_STATUS_SUSPENDED:
+ return "suspended";
case TRANSFER_STATUS_COMPLETE:
return "complete";
case TRANSFER_STATUS_ERROR:
@@ -300,6 +338,7 @@ static gboolean get_session(const GDBusPropertyTable *property,
}

static const GDBusMethodTable obc_transfer_methods[] = {
+ { GDBUS_METHOD("Suspend", NULL, NULL, obc_transfer_suspend) },
{ GDBUS_ASYNC_METHOD("Cancel", NULL, NULL,
obc_transfer_cancel) },
{ }
@@ -549,14 +588,6 @@ static gboolean get_xfer_progress(const void *buf, gsize len,
return TRUE;
}

-static void transfer_set_status(struct obc_transfer *transfer, uint8_t status)
-{
- transfer->status = status;
-
- g_dbus_emit_property_changed(transfer->conn, transfer->path,
- TRANSFER_INTERFACE, "Status");
-}
-
static void xfer_complete(GObex *obex, GError *err, gpointer user_data)
{
struct obc_transfer *transfer = user_data;
@@ -634,13 +665,15 @@ static void get_xfer_progress_first(GObex *obex, GError *err, GObexPacket *rsp,
return;
}

- if (!g_obex_srm_active(obex)) {
- req = g_obex_packet_new(G_OBEX_OP_GET, TRUE, G_OBEX_HDR_INVALID);
+ if (g_obex_srm_active(obex) ||
+ transfer->status == TRANSFER_STATUS_SUSPENDED)
+ return;

- transfer->xfer = g_obex_get_req_pkt(obex, req, get_xfer_progress,
+ req = g_obex_packet_new(G_OBEX_OP_GET, TRUE, G_OBEX_HDR_INVALID);
+
+ transfer->xfer = g_obex_get_req_pkt(obex, req, get_xfer_progress,
xfer_complete, transfer,
&err);
- }
}

static gssize put_xfer_progress(void *buf, gsize len, gpointer user_data)
@@ -689,7 +722,8 @@ static gboolean report_progress(gpointer data)
return FALSE;
}

- if (transfer->status != TRANSFER_STATUS_ACTIVE)
+ if (transfer->status != TRANSFER_STATUS_ACTIVE &&
+ transfer->status != TRANSFER_STATUS_SUSPENDED)
transfer_set_status(transfer, TRANSFER_STATUS_ACTIVE);

g_dbus_emit_property_changed(transfer->conn, transfer->path,
--
1.8.5.3



2014-02-20 12:09:15

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 8/8] doc/obex-api: Update documentation

Hi Patrick,

On Thu, Feb 20, 2014 at 12:52 PM, Patrick Ohly <[email protected]> wrote:
> On Wed, 2014-02-19 at 19:11 +0200, Luiz Augusto von Dentz wrote:
>> Hi Patrick,
>>
>> On Tue, Feb 18, 2014 at 6:54 PM, Patrick Ohly <[email protected]> wrote:
>> > On Tue, 2014-02-18 at 11:30 +0200, Luiz Augusto von Dentz wrote:
>> >> Hi Patrick,
>> >>
>> >> On Tue, Feb 18, 2014 at 11:09 AM, Patrick Ohly <[email protected]> wrote:
>> > I think I would prefer to *not* get errors when Suspend() is called on
>> > an already suspended transfer or when Resume() is called on a running
>> > transfer. The rationale is that the caller will typically only care
>> > about the end result and not so much whether it was the one who
>> > triggered the change. If we remove NotInProgress resp. InProgress errors
>> > from the API, then such a caller can treat all errors as fatal problem,
>> > without having to check the exact error.
>>
>> Actually the NotInProgress error is for transfer queued, I guess for
>> this case it makes sense to return an error since otherwise we would
>> have to queue commands to be send when the transfer becomes active I
>> believe this should probably be considered an error.
>
> Agreed, if suspending can't be done, then an error needs to be returned.
>
> It would be good to add information about the error to the
> documentation, so that apps get at least some hint what they are
> supposed to do in case of such an error (like aborting the queued
> transfer (most likely) or waiting for it to start before suspending it
> (seems a bit odd, but perhaps it is useful sometimes)).

I went ahead and pushed with a note about NotInProgress error, if we
figure that is not enough we can reword it to be more clear, but at
least in terms of API and functionality it should be fine.


--
Luiz Augusto von Dentz

2014-02-20 10:52:03

by Patrick Ohly

[permalink] [raw]
Subject: Re: [PATCH BlueZ 8/8] doc/obex-api: Update documentation

On Wed, 2014-02-19 at 19:11 +0200, Luiz Augusto von Dentz wrote:
> Hi Patrick,
>
> On Tue, Feb 18, 2014 at 6:54 PM, Patrick Ohly <[email protected]> wrote:
> > On Tue, 2014-02-18 at 11:30 +0200, Luiz Augusto von Dentz wrote:
> >> Hi Patrick,
> >>
> >> On Tue, Feb 18, 2014 at 11:09 AM, Patrick Ohly <[email protected]> wrote:
> > I think I would prefer to *not* get errors when Suspend() is called on
> > an already suspended transfer or when Resume() is called on a running
> > transfer. The rationale is that the caller will typically only care
> > about the end result and not so much whether it was the one who
> > triggered the change. If we remove NotInProgress resp. InProgress errors
> > from the API, then such a caller can treat all errors as fatal problem,
> > without having to check the exact error.
>
> Actually the NotInProgress error is for transfer queued, I guess for
> this case it makes sense to return an error since otherwise we would
> have to queue commands to be send when the transfer becomes active I
> believe this should probably be considered an error.

Agreed, if suspending can't be done, then an error needs to be returned.

It would be good to add information about the error to the
documentation, so that apps get at least some hint what they are
supposed to do in case of such an error (like aborting the queued
transfer (most likely) or waiting for it to start before suspending it
(seems a bit odd, but perhaps it is useful sometimes)).

--
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.




2014-02-19 17:11:41

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 8/8] doc/obex-api: Update documentation

Hi Patrick,

On Tue, Feb 18, 2014 at 6:54 PM, Patrick Ohly <[email protected]> wrote:
> On Tue, 2014-02-18 at 11:30 +0200, Luiz Augusto von Dentz wrote:
>> Hi Patrick,
>>
>> On Tue, Feb 18, 2014 at 11:09 AM, Patrick Ohly <[email protected]> wrote:
>> > On Fri, 2014-02-14 at 17:53 +0200, Luiz Augusto von Dentz wrote:
>> >> From: Luiz Augusto von Dentz <[email protected]>
>> >>
>> >> This adds Suspend and Resume methods and 'suspended' value as status in
>> >> the Transfer interface documentation.
>> >> ---
>> >> doc/obex-api.txt | 18 ++++++++++++++++--
>> >> 1 file changed, 16 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
>> >> index 1f22fea..0f57ce1 100644
>> >> --- a/doc/obex-api.txt
>> >> +++ b/doc/obex-api.txt
>> >> @@ -90,12 +90,26 @@ Methods void Cancel()
>> >> org.bluez.obex.Error.InProgress
>> >> org.bluez.obex.Error.Failed
>> >>
>> >> + void Suspend()
>> >> +
>> >> + Suspend transference.
>> >> +
>> >> + Possible errors: org.bluez.obex.Error.NotAuthorized
>> >> + org.bluez.obex.Error.NotInProgress
>> >> +
>> >> + void Resume()
>> >> +
>> >> + Resume transference.
>> >> +
>> >> + Possible errors: org.bluez.obex.Error.NotAuthorized
>> >> + org.bluez.obex.Error.NotInProgress
>> > ^^^^^^^^^^^^^
>> >
>> > Should this be a NotSuspended error? Or is it not an error to resume a
>> > transfer which is currently not suspended?
>>
>> Hmm, I would go for InProgress like in Cancel although now that you
>> mentioned we could perhaps ignore such errors if you don't see any
>> value on those.
>
> I think I would prefer to *not* get errors when Suspend() is called on
> an already suspended transfer or when Resume() is called on a running
> transfer. The rationale is that the caller will typically only care
> about the end result and not so much whether it was the one who
> triggered the change. If we remove NotInProgress resp. InProgress errors
> from the API, then such a caller can treat all errors as fatal problem,
> without having to check the exact error.

Actually the NotInProgress error is for transfer queued, I guess for
this case it makes sense to return an error since otherwise we would
have to queue commands to be send when the transfer becomes active I
believe this should probably be considered an error.


--
Luiz Augusto von Dentz

2014-02-18 16:54:21

by Patrick Ohly

[permalink] [raw]
Subject: Re: [PATCH BlueZ 8/8] doc/obex-api: Update documentation

On Tue, 2014-02-18 at 11:30 +0200, Luiz Augusto von Dentz wrote:
> Hi Patrick,
>
> On Tue, Feb 18, 2014 at 11:09 AM, Patrick Ohly <[email protected]> wrote:
> > On Fri, 2014-02-14 at 17:53 +0200, Luiz Augusto von Dentz wrote:
> >> From: Luiz Augusto von Dentz <[email protected]>
> >>
> >> This adds Suspend and Resume methods and 'suspended' value as status in
> >> the Transfer interface documentation.
> >> ---
> >> doc/obex-api.txt | 18 ++++++++++++++++--
> >> 1 file changed, 16 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
> >> index 1f22fea..0f57ce1 100644
> >> --- a/doc/obex-api.txt
> >> +++ b/doc/obex-api.txt
> >> @@ -90,12 +90,26 @@ Methods void Cancel()
> >> org.bluez.obex.Error.InProgress
> >> org.bluez.obex.Error.Failed
> >>
> >> + void Suspend()
> >> +
> >> + Suspend transference.
> >> +
> >> + Possible errors: org.bluez.obex.Error.NotAuthorized
> >> + org.bluez.obex.Error.NotInProgress
> >> +
> >> + void Resume()
> >> +
> >> + Resume transference.
> >> +
> >> + Possible errors: org.bluez.obex.Error.NotAuthorized
> >> + org.bluez.obex.Error.NotInProgress
> > ^^^^^^^^^^^^^
> >
> > Should this be a NotSuspended error? Or is it not an error to resume a
> > transfer which is currently not suspended?
>
> Hmm, I would go for InProgress like in Cancel although now that you
> mentioned we could perhaps ignore such errors if you don't see any
> value on those.

I think I would prefer to *not* get errors when Suspend() is called on
an already suspended transfer or when Resume() is called on a running
transfer. The rationale is that the caller will typically only care
about the end result and not so much whether it was the one who
triggered the change. If we remove NotInProgress resp. InProgress errors
from the API, then such a caller can treat all errors as fatal problem,
without having to check the exact error.

Bye, Patrick










2014-02-18 09:30:22

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 8/8] doc/obex-api: Update documentation

Hi Patrick,

On Tue, Feb 18, 2014 at 11:09 AM, Patrick Ohly <[email protected]> wrote:
> On Fri, 2014-02-14 at 17:53 +0200, Luiz Augusto von Dentz wrote:
>> From: Luiz Augusto von Dentz <[email protected]>
>>
>> This adds Suspend and Resume methods and 'suspended' value as status in
>> the Transfer interface documentation.
>> ---
>> doc/obex-api.txt | 18 ++++++++++++++++--
>> 1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
>> index 1f22fea..0f57ce1 100644
>> --- a/doc/obex-api.txt
>> +++ b/doc/obex-api.txt
>> @@ -90,12 +90,26 @@ Methods void Cancel()
>> org.bluez.obex.Error.InProgress
>> org.bluez.obex.Error.Failed
>>
>> + void Suspend()
>> +
>> + Suspend transference.
>> +
>> + Possible errors: org.bluez.obex.Error.NotAuthorized
>> + org.bluez.obex.Error.NotInProgress
>> +
>> + void Resume()
>> +
>> + Resume transference.
>> +
>> + Possible errors: org.bluez.obex.Error.NotAuthorized
>> + org.bluez.obex.Error.NotInProgress
> ^^^^^^^^^^^^^
>
> Should this be a NotSuspended error? Or is it not an error to resume a
> transfer which is currently not suspended?

Hmm, I would go for InProgress like in Cancel although now that you
mentioned we could perhaps ignore such errors if you don't see any
value on those.

--
Luiz Augusto von Dentz

2014-02-18 09:09:11

by Patrick Ohly

[permalink] [raw]
Subject: Re: [PATCH BlueZ 8/8] doc/obex-api: Update documentation

On Fri, 2014-02-14 at 17:53 +0200, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This adds Suspend and Resume methods and 'suspended' value as status in
> the Transfer interface documentation.
> ---
> doc/obex-api.txt | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
> index 1f22fea..0f57ce1 100644
> --- a/doc/obex-api.txt
> +++ b/doc/obex-api.txt
> @@ -90,12 +90,26 @@ Methods void Cancel()
> org.bluez.obex.Error.InProgress
> org.bluez.obex.Error.Failed
>
> + void Suspend()
> +
> + Suspend transference.
> +
> + Possible errors: org.bluez.obex.Error.NotAuthorized
> + org.bluez.obex.Error.NotInProgress
> +
> + void Resume()
> +
> + Resume transference.
> +
> + Possible errors: org.bluez.obex.Error.NotAuthorized
> + org.bluez.obex.Error.NotInProgress
^^^^^^^^^^^^^

Should this be a NotSuspended error? Or is it not an error to resume a
transfer which is currently not suspended?

--
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.



2014-02-17 07:24:41

by Andrei Emeltchenko

[permalink] [raw]
Subject: Re: [PATCH BlueZ 8/8] doc/obex-api: Update documentation

Hi Luiz,

On Fri, Feb 14, 2014 at 05:53:04PM +0200, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This adds Suspend and Resume methods and 'suspended' value as status in
> the Transfer interface documentation.
> ---
> doc/obex-api.txt | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/doc/obex-api.txt b/doc/obex-api.txt
> index 1f22fea..0f57ce1 100644
> --- a/doc/obex-api.txt
> +++ b/doc/obex-api.txt
> @@ -90,12 +90,26 @@ Methods void Cancel()
> org.bluez.obex.Error.InProgress
> org.bluez.obex.Error.Failed
>
> + void Suspend()
> +
> + Suspend transference.

would transfer sound better then transference ?

Best regards
Andrei Emeltchenko

> +
> + Possible errors: org.bluez.obex.Error.NotAuthorized
> + org.bluez.obex.Error.NotInProgress
> +
> + void Resume()
> +
> + Resume transference.
> +
> + Possible errors: org.bluez.obex.Error.NotAuthorized
> + org.bluez.obex.Error.NotInProgress
> +
> Properties string Status [readonly]
>
> Inform the current status of the transfer.
>
> - Possible values: "queued", "active", "complete" or
> - "error"
> + Possible values: "queued", "active", "suspended",
> + "complete" or "error"
>
> object Session [readonly]
>
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-02-14 15:53:04

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 8/8] doc/obex-api: Update documentation

From: Luiz Augusto von Dentz <[email protected]>

This adds Suspend and Resume methods and 'suspended' value as status in
the Transfer interface documentation.
---
doc/obex-api.txt | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/doc/obex-api.txt b/doc/obex-api.txt
index 1f22fea..0f57ce1 100644
--- a/doc/obex-api.txt
+++ b/doc/obex-api.txt
@@ -90,12 +90,26 @@ Methods void Cancel()
org.bluez.obex.Error.InProgress
org.bluez.obex.Error.Failed

+ void Suspend()
+
+ Suspend transference.
+
+ Possible errors: org.bluez.obex.Error.NotAuthorized
+ org.bluez.obex.Error.NotInProgress
+
+ void Resume()
+
+ Resume transference.
+
+ Possible errors: org.bluez.obex.Error.NotAuthorized
+ org.bluez.obex.Error.NotInProgress
+
Properties string Status [readonly]

Inform the current status of the transfer.

- Possible values: "queued", "active", "complete" or
- "error"
+ Possible values: "queued", "active", "suspended",
+ "complete" or "error"

object Session [readonly]

--
1.8.5.3


2014-02-14 15:53:03

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 7/8] gobex: Handle suspending/resuming for GET when SRM is active

From: Luiz Augusto von Dentz <[email protected]>

This adds support for suspending/resuming GET requests GET when SRM is
active, in this case suspending the TX queue wont stop the remote
to continue sending packets, to do that SRMP header should be set to wait
so the remote should wait.
---
gobex/gobex-transfer.c | 2 +-
gobex/gobex.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/gobex/gobex-transfer.c b/gobex/gobex-transfer.c
index b815d60..09f56ba 100644
--- a/gobex/gobex-transfer.c
+++ b/gobex/gobex-transfer.c
@@ -390,7 +390,7 @@ static void transfer_put_req(GObex *obex, GObexPacket *req, gpointer user_data)

rspcode = put_get_bytes(transfer, req);

- /* Don't send continue while in SRM */
+ /* Don't send continue while SRM is active */
if (g_obex_srm_active(transfer->obex) &&
rspcode == G_OBEX_RSP_CONTINUE)
goto done;
diff --git a/gobex/gobex.c b/gobex/gobex.c
index 291ed72..0d9b449 100644
--- a/gobex/gobex.c
+++ b/gobex/gobex.c
@@ -105,6 +105,7 @@ struct pending_pkt {
GObexResponseFunc rsp_func;
gpointer rsp_data;
gboolean cancelled;
+ gboolean suspended;
};

struct req_handler {
@@ -407,7 +408,9 @@ static void setup_srm(GObex *obex, GObexPacket *pkt, gboolean outgoing)
g_obex_header_get_uint8(hdr, &srmp);
g_obex_debug(G_OBEX_DEBUG_COMMAND, "srmp 0x%02x", srmp);
set_srmp(obex, srmp, outgoing);
- } else
+ } else if (obex->pending_req && obex->pending_req->suspended)
+ g_obex_packet_add_uint8(pkt, G_OBEX_HDR_SRMP, G_OBEX_SRMP_WAIT);
+ else
set_srmp(obex, -1, outgoing);

if (final)
@@ -845,24 +848,73 @@ gboolean g_obex_remove_request_function(GObex *obex, guint id)
return TRUE;
}

+static void g_obex_srm_suspend(GObex *obex)
+{
+ struct pending_pkt *p = obex->pending_req;
+ GObexPacket *req;
+
+ g_source_remove(p->timeout_id);
+ p->suspended = TRUE;
+
+ req = g_obex_packet_new(G_OBEX_OP_GET, TRUE,
+ G_OBEX_HDR_SRMP, G_OBEX_SRMP_WAIT,
+ G_OBEX_HDR_INVALID);
+
+ g_obex_send(obex, req, NULL);
+}
+
void g_obex_suspend(GObex *obex)
{
+ struct pending_pkt *req = obex->pending_req;
+
g_obex_debug(G_OBEX_DEBUG_COMMAND, "conn %u", obex->conn_id);

+ if (!g_obex_srm_active(obex) || !req)
+ goto done;
+
+ /* Send SRMP wait in case of GET */
+ if (g_obex_packet_get_operation(req->pkt, NULL) == G_OBEX_OP_GET) {
+ g_obex_srm_suspend(obex);
+ return;
+ }
+
+done:
+ obex->suspended = TRUE;
+
if (obex->write_source > 0) {
g_source_remove(obex->write_source);
obex->write_source = 0;
}
+}

- obex->suspended = TRUE;
+static void g_obex_srm_resume(GObex *obex)
+{
+ struct pending_pkt *p = obex->pending_req;
+ GObexPacket *req;
+
+ p->timeout_id = g_timeout_add_seconds(p->timeout, req_timeout, obex);
+ p->suspended = FALSE;
+
+ req = g_obex_packet_new(G_OBEX_OP_GET, TRUE, G_OBEX_HDR_INVALID);
+
+ g_obex_send(obex, req, NULL);
}

void g_obex_resume(GObex *obex)
{
+ struct pending_pkt *req = obex->pending_req;
+
g_obex_debug(G_OBEX_DEBUG_COMMAND, "conn %u", obex->conn_id);

obex->suspended = FALSE;

+ if (g_obex_srm_active(obex) || !req)
+ goto done;
+
+ if (g_obex_packet_get_operation(req->pkt, NULL) == G_OBEX_OP_GET)
+ g_obex_srm_resume(obex);
+
+done:
if (g_queue_get_length(obex->tx_queue) > 0 || obex->tx_data > 0)
enable_tx(obex);
}
--
1.8.5.3


2014-02-14 15:53:02

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 6/8] gobex: Fix not handling SRM properly

From: Luiz Augusto von Dentz <[email protected]>

SRM can be enabled but while not active with use of SRMP header so the
handling of this states needs to be separated.
---
gobex/gobex.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/gobex/gobex.c b/gobex/gobex.c
index 8c08b1e..291ed72 100644
--- a/gobex/gobex.c
+++ b/gobex/gobex.c
@@ -355,9 +355,20 @@ done:
obex->srm = NULL;
}

+static gboolean g_obex_srm_enabled(GObex *obex)
+{
+ if (!obex->use_srm)
+ return FALSE;
+
+ if (obex->srm == NULL)
+ return FALSE;
+
+ return obex->srm->enabled;
+}
+
static void check_srm_final(GObex *obex, guint8 op)
{
- if (obex->srm == NULL || !obex->srm->enabled)
+ if (!g_obex_srm_enabled(obex))
return;

switch (obex->srm->op) {
@@ -423,7 +434,7 @@ static gboolean write_data(GIOChannel *io, GIOCondition cond,

setup_srm(obex, p->pkt, TRUE);

- if (g_obex_srm_active(obex))
+ if (g_obex_srm_enabled(obex))
goto encode;

/* Can't send a request while there's a pending one */
@@ -646,7 +657,7 @@ guint g_obex_send_req(GObex *obex, GObexPacket *req, int timeout,
if (obex->rx_last_op == G_OBEX_RSP_CONTINUE)
goto create_pending;

- if (g_obex_srm_active(obex) && obex->pending_req != NULL)
+ if (g_obex_srm_enabled(obex) && obex->pending_req != NULL)
goto create_pending;

hdr = g_obex_packet_get_header(req, G_OBEX_HDR_CONNECTION);
@@ -860,10 +871,7 @@ gboolean g_obex_srm_active(GObex *obex)
{
gboolean ret = FALSE;

- if (!obex->use_srm)
- return FALSE;
-
- if (obex->srm == NULL || !obex->srm->enabled)
+ if (!g_obex_srm_enabled(obex))
goto done;

if (obex->srm->srmp <= G_OBEX_SRMP_NEXT_WAIT)
@@ -912,7 +920,7 @@ static gboolean parse_response(GObex *obex, GObexPacket *rsp)

setup_srm(obex, rsp, FALSE);

- if (!g_obex_srm_active(obex))
+ if (!g_obex_srm_enabled(obex))
return final;

/*
--
1.8.5.3


2014-02-14 15:52:59

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 3/8] tools/obexctl: Add suspend command

From: Luiz Augusto von Dentz <[email protected]>

Add support for suspend command which can be used to suspend ongoing
transfers.
---
tools/obexctl.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)

diff --git a/tools/obexctl.c b/tools/obexctl.c
index f0d5438..000e44d 100644
--- a/tools/obexctl.c
+++ b/tools/obexctl.c
@@ -596,6 +596,46 @@ static void cmd_cancel(int argc, char *argv[])
g_dbus_proxy_get_path(proxy));
}

+static void suspend_reply(DBusMessage *message, void *user_data)
+{
+ DBusError error;
+
+ dbus_error_init(&error);
+
+ if (dbus_set_error_from_message(&error, message) == TRUE) {
+ rl_printf("Failed to suspend: %s\n", error.name);
+ dbus_error_free(&error);
+ return;
+ }
+
+ rl_printf("Suspend successful\n");
+}
+
+static void cmd_suspend(int argc, char *argv[])
+{
+ GDBusProxy *proxy;
+
+ if (argc < 2) {
+ rl_printf("Missing transfer address argument\n");
+ return;
+ }
+
+ proxy = find_transfer(argv[1]);
+ if (!proxy) {
+ rl_printf("Transfer %s not available\n", argv[1]);
+ return;
+ }
+
+ if (g_dbus_proxy_method_call(proxy, "Suspend", NULL, suspend_reply,
+ NULL, NULL) == FALSE) {
+ rl_printf("Failed to suspend transfer\n");
+ return;
+ }
+
+ rl_printf("Attempting to suspend transfer %s\n",
+ g_dbus_proxy_get_path(proxy));
+}
+
static GDBusProxy *find_opp(const char *path)
{
GSList *l;
@@ -1856,6 +1896,7 @@ static const struct {
{ "select", "<session>", cmd_select, "Select default session" },
{ "info", "<object>", cmd_info, "Object information" },
{ "cancel", "<transfer>", cmd_cancel, "Cancel transfer" },
+ { "suspend", "<transfer>", cmd_suspend, "Suspend transfer" },
{ "send", "<file>", cmd_send, "Send file" },
{ "cd", "<path>", cmd_cd, "Change current folder" },
{ "ls", NULL, cmd_ls, "List current folder" },
--
1.8.5.3


2014-02-14 15:53:01

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 5/8] unit/test-gobex-transfer: Add /gobex/test_packet_get_req_suspend_resume

From: Luiz Augusto von Dentz <[email protected]>

This adds a test to call g_obex_suspend and latter g_obex_resume while
SRM is enabled.
---
unit/test-gobex-transfer.c | 81 +++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 77 insertions(+), 4 deletions(-)

diff --git a/unit/test-gobex-transfer.c b/unit/test-gobex-transfer.c
index 7c9fd43..0d26111 100644
--- a/unit/test-gobex-transfer.c
+++ b/unit/test-gobex-transfer.c
@@ -106,6 +106,9 @@ static guint8 get_req_first_srm_wait[] = { G_OBEX_OP_GET | FINAL_BIT, 0x00, 0x27
0, 'f', 0, 'i', 0, 'l', 0, 'e', 0, '.', 0, 't', 0, 'x', 0, 't', 0, 0,
G_OBEX_HDR_SRMP, 0x01 };

+static guint8 get_req_srm_wait[] = { G_OBEX_OP_GET | FINAL_BIT, 0x00, 0x05,
+ G_OBEX_HDR_SRMP, 0x01 };
+
static guint8 get_req_last[] = { G_OBEX_OP_GET | FINAL_BIT, 0x00, 0x03, };

static guint8 get_rsp_first_app[] = { G_OBEX_RSP_CONTINUE | FINAL_BIT, 0x00, 0x0A,
@@ -124,6 +127,8 @@ static guint8 get_rsp_first_srm_wait_next[] = { G_OBEX_RSP_CONTINUE | FINAL_BIT,
G_OBEX_HDR_SRMP, 0x02,
G_OBEX_HDR_BODY, 0x00, 0x0d,
0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+static guint8 get_rsp_srm_wait[] = { G_OBEX_RSP_CONTINUE | FINAL_BIT,
+ 0x00, 0x03 };
static guint8 get_rsp_zero[255] = { G_OBEX_RSP_CONTINUE | FINAL_BIT, 0x00, 0xff,
G_OBEX_HDR_BODY, 0x00, 0xfc };
static guint8 get_rsp_zero_wait_next[255] = { G_OBEX_RSP_CONTINUE | FINAL_BIT,
@@ -191,7 +196,13 @@ static void transfer_complete(GObex *obex, GError *err, gpointer user_data)

static gboolean resume_obex(gpointer user_data)
{
- g_obex_resume(user_data);
+ struct test_data *d = user_data;
+
+ if (!g_main_loop_is_running(d->mainloop))
+ return FALSE;
+
+ g_obex_resume(d->obex);
+
return FALSE;
}

@@ -232,7 +243,7 @@ static gssize provide_eagain(void *buf, gsize len, gpointer user_data)
}

if (d->provide_delay > 0) {
- g_timeout_add(d->provide_delay, resume_obex, d->obex);
+ g_timeout_add(d->provide_delay, resume_obex, d);
d->provide_delay = 0;
return -EAGAIN;
}
@@ -260,7 +271,7 @@ static gssize provide_data(void *buf, gsize len, gpointer user_data)

if (d->provide_delay > 0) {
g_obex_suspend(d->obex);
- g_timeout_add(d->provide_delay, resume_obex, d->obex);
+ g_timeout_add(d->provide_delay, resume_obex, d);
}

d->total += sizeof(body_data);
@@ -852,6 +863,66 @@ static void test_packet_get_req_wait(void)
g_assert_no_error(d.err);
}

+static gboolean rcv_seq_delay(const void *buf, gsize len, gpointer user_data)
+{
+ struct test_data *d = user_data;
+
+ if (d->provide_delay > 0) {
+ g_obex_suspend(d->obex);
+ g_idle_add(resume_obex, d);
+ d->provide_delay = 0;
+ }
+
+ return TRUE;
+}
+
+static void test_packet_get_req_suspend_resume(void)
+{
+ GIOChannel *io;
+ GIOCondition cond;
+ guint io_id, timer_id;
+ GObex *obex;
+ struct test_data d = { 0, NULL, {
+ { get_req_first_srm, sizeof(get_req_first_srm) },
+ { get_req_srm_wait, sizeof(get_req_srm_wait) },
+ { get_req_srm_wait, sizeof(get_req_srm_wait) },
+ { get_req_last, sizeof(get_req_last) } }, {
+ { get_rsp_first_srm, sizeof(get_rsp_first_srm) },
+ { get_rsp_srm_wait, sizeof(get_rsp_srm_wait) },
+ { get_rsp_srm_wait, sizeof(get_rsp_srm_wait) },
+ { get_rsp_last, sizeof(get_rsp_last) } } };
+
+ create_endpoints(&obex, &io, SOCK_SEQPACKET);
+ d.obex = obex;
+ d.provide_delay = 1;
+
+ cond = G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL;
+ io_id = g_io_add_watch(io, cond, test_io_cb, &d);
+
+ d.mainloop = g_main_loop_new(NULL, FALSE);
+
+ timer_id = g_timeout_add_seconds(1, test_timeout, &d);
+
+ g_obex_get_req(obex, rcv_seq_delay, transfer_complete, &d, &d.err,
+ G_OBEX_HDR_TYPE, hdr_type, sizeof(hdr_type),
+ G_OBEX_HDR_NAME, "file.txt",
+ G_OBEX_HDR_INVALID);
+ g_assert_no_error(d.err);
+
+ g_main_loop_run(d.mainloop);
+
+ g_assert_cmpuint(d.count, ==, 4);
+
+ g_main_loop_unref(d.mainloop);
+
+ g_source_remove(timer_id);
+ g_io_channel_unref(io);
+ g_source_remove(io_id);
+ g_obex_unref(obex);
+
+ g_assert_no_error(d.err);
+}
+
static void test_packet_get_req_wait_next(void)
{
GIOChannel *io;
@@ -1549,7 +1620,7 @@ static gboolean rcv_data_delay(const void *buf, gsize len, gpointer user_data)

if (d->provide_delay > 0) {
g_obex_suspend(d->obex);
- g_timeout_add(d->provide_delay, resume_obex, d->obex);
+ g_timeout_add(d->provide_delay, resume_obex, d);
}

return TRUE;
@@ -2253,6 +2324,8 @@ int main(int argc, char *argv[])
g_test_add_func("/gobex/test_packet_get_req", test_packet_get_req);
g_test_add_func("/gobex/test_packet_get_req_wait",
test_packet_get_req_wait);
+ g_test_add_func("/gobex/test_packet_get_req_suspend_resume",
+ test_packet_get_req_suspend_resume);

g_test_add_func("/gobex/test_packet_get_req_wait_next",
test_packet_get_req_wait_next);
--
1.8.5.3


2014-02-14 15:53:00

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 4/8] tools/obexctl: Add resume command

From: Luiz Augusto von Dentz <[email protected]>

Add support for resume command which can be used to resume ongoing
transfers.
---
tools/obexctl.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)

diff --git a/tools/obexctl.c b/tools/obexctl.c
index 000e44d..fc051d7 100644
--- a/tools/obexctl.c
+++ b/tools/obexctl.c
@@ -636,6 +636,46 @@ static void cmd_suspend(int argc, char *argv[])
g_dbus_proxy_get_path(proxy));
}

+static void resume_reply(DBusMessage *message, void *user_data)
+{
+ DBusError error;
+
+ dbus_error_init(&error);
+
+ if (dbus_set_error_from_message(&error, message) == TRUE) {
+ rl_printf("Failed to resume: %s\n", error.name);
+ dbus_error_free(&error);
+ return;
+ }
+
+ rl_printf("Resume successful\n");
+}
+
+static void cmd_resume(int argc, char *argv[])
+{
+ GDBusProxy *proxy;
+
+ if (argc < 2) {
+ rl_printf("Missing transfer address argument\n");
+ return;
+ }
+
+ proxy = find_transfer(argv[1]);
+ if (!proxy) {
+ rl_printf("Transfer %s not available\n", argv[1]);
+ return;
+ }
+
+ if (g_dbus_proxy_method_call(proxy, "Resume", NULL, resume_reply,
+ NULL, NULL) == FALSE) {
+ rl_printf("Failed to resume transfer\n");
+ return;
+ }
+
+ rl_printf("Attempting to resume transfer %s\n",
+ g_dbus_proxy_get_path(proxy));
+}
+
static GDBusProxy *find_opp(const char *path)
{
GSList *l;
@@ -1897,6 +1937,7 @@ static const struct {
{ "info", "<object>", cmd_info, "Object information" },
{ "cancel", "<transfer>", cmd_cancel, "Cancel transfer" },
{ "suspend", "<transfer>", cmd_suspend, "Suspend transfer" },
+ { "resume", "<transfer>", cmd_resume, "Resume transfer" },
{ "send", "<file>", cmd_send, "Send file" },
{ "cd", "<path>", cmd_cd, "Change current folder" },
{ "ls", NULL, cmd_ls, "List current folder" },
--
1.8.5.3


2014-02-14 15:52:58

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 2/8] obexd/transfer: Add Transfer.Resume method

From: Luiz Augusto von Dentz <[email protected]>

This method can be used to resume ongoing transfers.
---
obexd/client/transfer.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/obexd/client/transfer.c b/obexd/client/transfer.c
index 813d20b..6295b83 100644
--- a/obexd/client/transfer.c
+++ b/obexd/client/transfer.c
@@ -223,6 +223,30 @@ static DBusMessage *obc_transfer_suspend(DBusConnection *connection,
return g_dbus_create_reply(message, DBUS_TYPE_INVALID);
}

+static DBusMessage *obc_transfer_resume(DBusConnection *connection,
+ DBusMessage *message, void *user_data)
+{
+ struct obc_transfer *transfer = user_data;
+ const char *sender;
+
+ sender = dbus_message_get_sender(message);
+ if (g_strcmp0(transfer->owner, sender) != 0)
+ return g_dbus_create_error(message,
+ ERROR_INTERFACE ".NotAuthorized",
+ "Not Authorized");
+
+ if (transfer->xfer == 0)
+ return g_dbus_create_error(message,
+ ERROR_INTERFACE ".NotInProgress",
+ "Not in progress");
+
+ g_obex_resume(transfer->obex);
+
+ transfer_set_status(transfer, TRANSFER_STATUS_ACTIVE);
+
+ return g_dbus_create_reply(message, DBUS_TYPE_INVALID);
+}
+
static gboolean name_exists(const GDBusPropertyTable *property, void *data)
{
struct obc_transfer *transfer = data;
@@ -339,6 +363,7 @@ static gboolean get_session(const GDBusPropertyTable *property,

static const GDBusMethodTable obc_transfer_methods[] = {
{ GDBUS_METHOD("Suspend", NULL, NULL, obc_transfer_suspend) },
+ { GDBUS_METHOD("Resume", NULL, NULL, obc_transfer_resume) },
{ GDBUS_ASYNC_METHOD("Cancel", NULL, NULL,
obc_transfer_cancel) },
{ }
--
1.8.5.3