2018-02-09 09:10:57

by ERAMOTO Masaya

[permalink] [raw]
Subject: [PATCH BlueZ v2 1/4] obexd: Use emitting function wrapped for transfer

Also replaces emit_transfer_progress() with the new wrapper function
because the total/transferred arguments have not been used since commit
a5c2b6e8 ("obexd: Align client and server spec of org.bluez.obex.Transfer1")
and g_dbus_emit_property_changed_full() checks if the path argument is
NULL.
---
obexd/src/manager.c | 26 ++++++++++----------------
obexd/src/manager.h | 2 ++
2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/obexd/src/manager.c b/obexd/src/manager.c
index 70265e445..7ae8d7f83 100644
--- a/obexd/src/manager.c
+++ b/obexd/src/manager.c
@@ -533,12 +533,18 @@ void manager_cleanup(void)
dbus_connection_unref(connection);
}

+void manager_emit_transfer_property(struct obex_transfer *transfer,
+ char *name)
+{
+ g_dbus_emit_property_changed(connection, transfer->path,
+ TRANSFER_INTERFACE, name);
+}
+
void manager_emit_transfer_started(struct obex_transfer *transfer)
{
transfer->status = TRANSFER_STATUS_ACTIVE;

- g_dbus_emit_property_changed(connection, transfer->path,
- TRANSFER_INTERFACE, "Status");
+ manager_emit_transfer_property(transfer, "Status");
}

static void emit_transfer_completed(struct obex_transfer *transfer,
@@ -550,18 +556,7 @@ static void emit_transfer_completed(struct obex_transfer *transfer,
transfer->status = success ? TRANSFER_STATUS_COMPLETE :
TRANSFER_STATUS_ERROR;

- g_dbus_emit_property_changed(connection, transfer->path,
- TRANSFER_INTERFACE, "Status");
-}
-
-static void emit_transfer_progress(struct obex_transfer *transfer,
- uint32_t total, uint32_t transferred)
-{
- if (transfer->path == NULL)
- return;
-
- g_dbus_emit_property_changed(connection, transfer->path,
- TRANSFER_INTERFACE, "Transferred");
+ manager_emit_transfer_property(transfer, "Status");
}

static void transfer_free(struct obex_transfer *transfer)
@@ -783,8 +778,7 @@ void manager_unregister_session(struct obex_session *os)

void manager_emit_transfer_progress(struct obex_transfer *transfer)
{
- emit_transfer_progress(transfer, transfer->session->size,
- transfer->session->offset);
+ manager_emit_transfer_property(transfer, "Transferred");
}

void manager_emit_transfer_completed(struct obex_transfer *transfer)
diff --git a/obexd/src/manager.h b/obexd/src/manager.h
index d9781b201..d709af5a8 100644
--- a/obexd/src/manager.h
+++ b/obexd/src/manager.h
@@ -33,6 +33,8 @@ void manager_unregister_session(struct obex_session *os);

struct obex_transfer *manager_register_transfer(struct obex_session *os);
void manager_unregister_transfer(struct obex_transfer *transfer);
+void manager_emit_transfer_property(struct obex_transfer *transfer,
+ char *name);
void manager_emit_transfer_started(struct obex_transfer *transfer);
void manager_emit_transfer_progress(struct obex_transfer *transfer);
void manager_emit_transfer_completed(struct obex_transfer *transfer);
--
2.14.1



2018-02-12 11:48:35

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 1/4] obexd: Use emitting function wrapped for transfer

Hi Eramoto,

On Mon, Feb 12, 2018 at 8:24 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Eramoto,
>
> On Fri, Feb 9, 2018 at 7:10 AM, ERAMOTO Masaya
> <[email protected]> wrote:
>> Also replaces emit_transfer_progress() with the new wrapper function
>> because the total/transferred arguments have not been used since commit
>> a5c2b6e8 ("obexd: Align client and server spec of org.bluez.obex.Transfer1")
>> and g_dbus_emit_property_changed_full() checks if the path argument is
>> NULL.
>> ---
>> obexd/src/manager.c | 26 ++++++++++----------------
>> obexd/src/manager.h | 2 ++
>> 2 files changed, 12 insertions(+), 16 deletions(-)
>>
>> diff --git a/obexd/src/manager.c b/obexd/src/manager.c
>> index 70265e445..7ae8d7f83 100644
>> --- a/obexd/src/manager.c
>> +++ b/obexd/src/manager.c
>> @@ -533,12 +533,18 @@ void manager_cleanup(void)
>> dbus_connection_unref(connection);
>> }
>>
>> +void manager_emit_transfer_property(struct obex_transfer *transfer,
>> + char *name)
>> +{
>> + g_dbus_emit_property_changed(connection, transfer->path,
>> + TRANSFER_INTERFACE, name);
>> +}
>
> We better put a check if the transfer->path is not NULL, otherwise looks good.
>
>> void manager_emit_transfer_started(struct obex_transfer *transfer)
>> {
>> transfer->status = TRANSFER_STATUS_ACTIVE;
>>
>> - g_dbus_emit_property_changed(connection, transfer->path,
>> - TRANSFER_INTERFACE, "Status");
>> + manager_emit_transfer_property(transfer, "Status");
>> }
>>
>> static void emit_transfer_completed(struct obex_transfer *transfer,
>> @@ -550,18 +556,7 @@ static void emit_transfer_completed(struct obex_transfer *transfer,
>> transfer->status = success ? TRANSFER_STATUS_COMPLETE :
>> TRANSFER_STATUS_ERROR;
>>
>> - g_dbus_emit_property_changed(connection, transfer->path,
>> - TRANSFER_INTERFACE, "Status");
>> -}
>> -
>> -static void emit_transfer_progress(struct obex_transfer *transfer,
>> - uint32_t total, uint32_t transferred)
>> -{
>> - if (transfer->path == NULL)
>> - return;
>> -
>> - g_dbus_emit_property_changed(connection, transfer->path,
>> - TRANSFER_INTERFACE, "Transferred");
>> + manager_emit_transfer_property(transfer, "Status");
>> }
>>
>> static void transfer_free(struct obex_transfer *transfer)
>> @@ -783,8 +778,7 @@ void manager_unregister_session(struct obex_session *os)
>>
>> void manager_emit_transfer_progress(struct obex_transfer *transfer)
>> {
>> - emit_transfer_progress(transfer, transfer->session->size,
>> - transfer->session->offset);
>> + manager_emit_transfer_property(transfer, "Transferred");
>> }
>>
>> void manager_emit_transfer_completed(struct obex_transfer *transfer)
>> diff --git a/obexd/src/manager.h b/obexd/src/manager.h
>> index d9781b201..d709af5a8 100644
>> --- a/obexd/src/manager.h
>> +++ b/obexd/src/manager.h
>> @@ -33,6 +33,8 @@ void manager_unregister_session(struct obex_session *os);
>>
>> struct obex_transfer *manager_register_transfer(struct obex_session *os);
>> void manager_unregister_transfer(struct obex_transfer *transfer);
>> +void manager_emit_transfer_property(struct obex_transfer *transfer,
>> + char *name);
>> void manager_emit_transfer_started(struct obex_transfer *transfer);
>> void manager_emit_transfer_progress(struct obex_transfer *transfer);
>> void manager_emit_transfer_completed(struct obex_transfer *transfer);
>> --
>> 2.14.1
>>
>> --
>> 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

Applied, thanks.

--
Luiz Augusto von Dentz

2018-02-12 10:24:34

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 1/4] obexd: Use emitting function wrapped for transfer

Hi Eramoto,

On Fri, Feb 9, 2018 at 7:10 AM, ERAMOTO Masaya
<[email protected]> wrote:
> Also replaces emit_transfer_progress() with the new wrapper function
> because the total/transferred arguments have not been used since commit
> a5c2b6e8 ("obexd: Align client and server spec of org.bluez.obex.Transfer1")
> and g_dbus_emit_property_changed_full() checks if the path argument is
> NULL.
> ---
> obexd/src/manager.c | 26 ++++++++++----------------
> obexd/src/manager.h | 2 ++
> 2 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/obexd/src/manager.c b/obexd/src/manager.c
> index 70265e445..7ae8d7f83 100644
> --- a/obexd/src/manager.c
> +++ b/obexd/src/manager.c
> @@ -533,12 +533,18 @@ void manager_cleanup(void)
> dbus_connection_unref(connection);
> }
>
> +void manager_emit_transfer_property(struct obex_transfer *transfer,
> + char *name)
> +{
> + g_dbus_emit_property_changed(connection, transfer->path,
> + TRANSFER_INTERFACE, name);
> +}

We better put a check if the transfer->path is not NULL, otherwise looks good.

> void manager_emit_transfer_started(struct obex_transfer *transfer)
> {
> transfer->status = TRANSFER_STATUS_ACTIVE;
>
> - g_dbus_emit_property_changed(connection, transfer->path,
> - TRANSFER_INTERFACE, "Status");
> + manager_emit_transfer_property(transfer, "Status");
> }
>
> static void emit_transfer_completed(struct obex_transfer *transfer,
> @@ -550,18 +556,7 @@ static void emit_transfer_completed(struct obex_transfer *transfer,
> transfer->status = success ? TRANSFER_STATUS_COMPLETE :
> TRANSFER_STATUS_ERROR;
>
> - g_dbus_emit_property_changed(connection, transfer->path,
> - TRANSFER_INTERFACE, "Status");
> -}
> -
> -static void emit_transfer_progress(struct obex_transfer *transfer,
> - uint32_t total, uint32_t transferred)
> -{
> - if (transfer->path == NULL)
> - return;
> -
> - g_dbus_emit_property_changed(connection, transfer->path,
> - TRANSFER_INTERFACE, "Transferred");
> + manager_emit_transfer_property(transfer, "Status");
> }
>
> static void transfer_free(struct obex_transfer *transfer)
> @@ -783,8 +778,7 @@ void manager_unregister_session(struct obex_session *os)
>
> void manager_emit_transfer_progress(struct obex_transfer *transfer)
> {
> - emit_transfer_progress(transfer, transfer->session->size,
> - transfer->session->offset);
> + manager_emit_transfer_property(transfer, "Transferred");
> }
>
> void manager_emit_transfer_completed(struct obex_transfer *transfer)
> diff --git a/obexd/src/manager.h b/obexd/src/manager.h
> index d9781b201..d709af5a8 100644
> --- a/obexd/src/manager.h
> +++ b/obexd/src/manager.h
> @@ -33,6 +33,8 @@ void manager_unregister_session(struct obex_session *os);
>
> struct obex_transfer *manager_register_transfer(struct obex_session *os);
> void manager_unregister_transfer(struct obex_transfer *transfer);
> +void manager_emit_transfer_property(struct obex_transfer *transfer,
> + char *name);
> void manager_emit_transfer_started(struct obex_transfer *transfer);
> void manager_emit_transfer_progress(struct obex_transfer *transfer);
> void manager_emit_transfer_completed(struct obex_transfer *transfer);
> --
> 2.14.1
>
> --
> 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



--
Luiz Augusto von Dentz

2018-02-09 09:12:38

by ERAMOTO Masaya

[permalink] [raw]
Subject: [PATCH BlueZ v2 4/4] tools/obexctl: Fix display of size on completion

Outputs zero as the transferred size on completion of transfer as below:

[CHG] Transfer /org/bluez/obex/server/session3/transfer2 Transferred: 5339965 (@32KB/s 00:01)
[CHG] Transfer /org/bluez/obex/server/session3/transfer2 Transferred: 5372726 (@32KB/s 00:00)
[CHG] Transfer /org/bluez/obex/server/session3/transfer2 Transferred: 0 (@18446744073704178KB/s 00:00)
[CHG] Transfer /org/bluez/obex/server/session3/transfer2 Status: complete
---
tools/obexctl.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/tools/obexctl.c b/tools/obexctl.c
index 4e46c7541..2ef1c044e 100644
--- a/tools/obexctl.c
+++ b/tools/obexctl.c
@@ -1862,6 +1862,14 @@ static void print_transferred(struct transfer_data *data, const char *str,
int seconds, minutes;

dbus_message_iter_get_basic(iter, &valu64);
+
+ /*
+ * Use the file size to output the proper size/speed since obexd resets
+ * the current transferred size to zero on completion of transfer.
+ */
+ if (valu64 == 0)
+ valu64 = data->size;
+
speed = valu64 - data->transferred;
data->transferred = valu64;

--
2.14.1


2018-02-09 09:12:15

by ERAMOTO Masaya

[permalink] [raw]
Subject: [PATCH BlueZ v2 3/4] tools/obexctl: Fix retrieving non-existent iterator

If transfer is completed with one time file transfer,
transfer_property_changed() is called with the iter argument, which is
NULL, as below:

#0 transfer_property_changed (proxy=<optimized out>, name=<optimized out>, iter=0x0, user_data=0x5555557b9060) at tools/obexctl.c:1896
#1 0x000055555556e3cf in properties_changed (conn=<optimized out>, msg=<optimized out>, user_data=0x5555557c8300) at gdbus/client.c:433
#2 0x000055555556c830 in signal_filter (connection=0x5555557c65a0, message=0x5555557ccea0, user_data=0x5555557cdbf0) at gdbus/watch.c:407
#3 0x000055555556c291 in message_filter (connection=0x5555557c65a0, message=0x5555557ccea0, user_data=<optimized out>) at gdbus/watch.c:557
#4 0x00007ffff7888661 in dbus_connection_dispatch () from /lib/x86_64-linux-gnu/libdbus-1.so.3
#5 0x000055555556b010 in message_dispatch (data=0x5555557c65a0) at gdbus/mainloop.c:72
#6 0x00007ffff7b0be25 in g_main_context_dispatch () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#7 0x00007ffff7b0c1f0 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#8 0x00007ffff7b0c502 in g_main_loop_run () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#9 0x0000555555577594 in bt_shell_run () at src/shared/shell.c:833
#10 0x000055555556516d in main (argc=<optimized out>, argv=<optimized out>) at tools/obexctl.c:2116
---
tools/obexctl.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/tools/obexctl.c b/tools/obexctl.c
index 05bbd3e84..4e46c7541 100644
--- a/tools/obexctl.c
+++ b/tools/obexctl.c
@@ -1885,6 +1885,9 @@ static void transfer_property_changed(GDBusProxy *proxy, const char *name,
struct transfer_data *data = user_data;
char *str;

+ if (iter == NULL)
+ return;
+
str = proxy_description(proxy, "Transfer", COLORED_CHG);

if (strcmp(name, "Transferred") == 0) {
--
2.14.1


2018-02-09 09:11:41

by ERAMOTO Masaya

[permalink] [raw]
Subject: [PATCH BlueZ v2 2/4] obexd: Emit Size property of transfer after open()

Emits the Size property of transfer after calling ->open() because it
may update the size information.
---
obexd/src/obex.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/obexd/src/obex.c b/obexd/src/obex.c
index a4bae857f..ed219d3e7 100644
--- a/obexd/src/obex.c
+++ b/obexd/src/obex.c
@@ -779,6 +779,9 @@ int obex_put_stream_start(struct obex_session *os, const char *filename)
return err;
}

+ if (os->size != OBJECT_SIZE_DELETE && os->size != OBJECT_SIZE_UNKNOWN)
+ manager_emit_transfer_property(os->service_data, "Size");
+
os->path = g_strdup(filename);

return 0;
--
2.14.1