* [PATCH v4 2/8] stk: Introduce BIP command handlers
2011-05-20 16:26 [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Philippe Nunes
@ 2011-05-20 16:26 ` Philippe Nunes
2011-06-02 0:28 ` Denis Kenzior
2011-05-20 16:26 ` [PATCH v4 3/8] gprs: Add 'stk' gprs context type Philippe Nunes
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Philippe Nunes @ 2011-05-20 16:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 13837 bytes --]
---
src/stk.c | 426 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 425 insertions(+), 1 deletions(-)
diff --git a/src/stk.c b/src/stk.c
index 8214b65..fb376a6 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -41,6 +41,7 @@
#include "smsutil.h"
#include "stkutil.h"
#include "stkagent.h"
+#include "gprs.h"
#include "util.h"
static GSList *g_drivers = NULL;
@@ -79,6 +80,11 @@ struct ofono_stk {
__ofono_sms_sim_download_cb_t sms_pp_cb;
void *sms_pp_userdata;
+ struct stk_channel channel;
+ struct stk_channel_data rx_buffer;
+ struct stk_channel_data tx_buffer;
+ gboolean link_on_demand;
+ struct ofono_gprs *gprs;
};
struct envelope_op {
@@ -104,6 +110,13 @@ static void timers_update(struct ofono_stk *stk);
result.additional_len = sizeof(addn_info); \
result.additional = addn_info; \
+/*
+ * According the structure and coding of the Terminal response defined in
+ * TS 102 223 section 6.8, the maximum number of bytes possible for the channel
+ * data object is 243
+ */
+#define CHANNEL_DATA_OBJECT_MAX_LENGTH 243
+
static int stk_respond(struct ofono_stk *stk, struct stk_response *rsp,
ofono_stk_generic_cb_t cb)
{
@@ -474,12 +487,43 @@ static void emit_menu_changed(struct ofono_stk *stk)
g_dbus_send_message(conn, signal);
}
+static void stk_close_channel(struct ofono_stk *stk)
+{
+ /*
+ * TODO
+ * Deactivate and remove PDP context
+ * Send the Terminal Response once the PDP context is deactivated
+ */
+
+ /* Temporary implementation */
+ g_free(stk->rx_buffer.data.array);
+ g_free(stk->tx_buffer.data.array);
+ stk->rx_buffer.data.array = NULL;
+ stk->tx_buffer.data.array = NULL;
+
+ stk->channel.id = 0;
+ stk->channel.status = STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED;
+
+ if (stk->pending_cmd &&
+ stk->pending_cmd->type ==
+ STK_COMMAND_TYPE_CLOSE_CHANNEL)
+ send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
+}
+
static void user_termination_cb(enum stk_agent_result result, void *user_data)
{
struct ofono_stk *stk = user_data;
- if (result == STK_AGENT_RESULT_TERMINATE)
+ if (result != STK_AGENT_RESULT_TERMINATE)
+ return;
+
+ if (stk->pending_cmd) {
+ stk->cancel_cmd(stk);
send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
+ }
+
+ if (stk->channel.id)
+ stk_close_channel(stk);
}
static void stk_alpha_id_set(struct ofono_stk *stk,
@@ -510,6 +554,147 @@ static void stk_alpha_id_unset(struct ofono_stk *stk)
stk_agent_request_cancel(stk->current_agent);
}
+
+static void stk_open_channel(struct ofono_stk *stk)
+{
+ const struct stk_command_open_channel *oc;
+ struct stk_response rsp;
+ struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
+
+ if (stk->pending_cmd == NULL ||
+ stk->pending_cmd->type != STK_COMMAND_TYPE_OPEN_CHANNEL)
+ return;
+
+ oc = &stk->pending_cmd->open_channel;
+
+ memset(&rsp, 0, sizeof(rsp));
+ rsp.result.type = STK_RESULT_TYPE_SUCCESS;
+
+ stk->rx_buffer.data.array = g_try_malloc(oc->buf_size);
+ if (stk->rx_buffer.data.array == NULL) {
+ unsigned char no_cause_result[] = { 0x00 };
+
+ ADD_ERROR_RESULT(rsp.result, STK_RESULT_TYPE_TERMINAL_BUSY,
+ no_cause_result);
+ goto out;
+ }
+
+ stk->tx_buffer.data.array = g_try_malloc(oc->buf_size);
+ if (stk->tx_buffer.data.array == NULL) {
+ unsigned char no_cause_result[] = { 0x00 };
+
+ ADD_ERROR_RESULT(rsp.result, STK_RESULT_TYPE_TERMINAL_BUSY,
+ no_cause_result);
+ goto out;
+ }
+
+ stk->rx_buffer.data.len = oc->buf_size;
+ stk->rx_buffer.rx_remaining = 0;
+ stk->tx_buffer.data.len = oc->buf_size;
+ stk->tx_buffer.tx_avail = oc->buf_size;
+ stk->link_on_demand = (stk->pending_cmd->qualifier &
+ STK_OPEN_CHANNEL_FLAG_IMMEDIATE) ? FALSE : TRUE;
+
+ /*
+ * TODO
+ * Add a new primary PDP context based on the provided settings
+ * Send the Terminal Response or wait until the PDP context is activated
+ * in case of immediate link establishment not in background.
+ */
+out:
+ if (stk_respond(stk, &rsp, stk_command_cb))
+ stk_command_cb(&failure, stk);
+
+ if (rsp.result.type == STK_RESULT_TYPE_NOT_CAPABLE && stk->channel.id)
+ stk_close_channel(stk);
+}
+
+static void stk_send_data(struct ofono_stk *stk,
+ struct stk_common_byte_array data,
+ unsigned char qualifier)
+{
+ struct stk_response rsp;
+ struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
+ unsigned int offset;
+
+ memset(&rsp, 0, sizeof(rsp));
+ rsp.result.type = STK_RESULT_TYPE_SUCCESS;
+
+ if (data.len > stk->tx_buffer.data.len ||
+ data.len > stk->tx_buffer.tx_avail) {
+ rsp.result.type = STK_RESULT_TYPE_BIP_ERROR;
+ goto out;
+ }
+
+ if (data.len) {
+ offset = stk->tx_buffer.data.len - stk->tx_buffer.tx_avail;
+ memcpy(stk->tx_buffer.data.array + offset, data.array,
+ data.len);
+
+ stk->tx_buffer.tx_avail -= data.len;
+
+ if (qualifier == STK_SEND_DATA_STORE_DATA) {
+ rsp.send_data.tx_avail = stk->tx_buffer.tx_avail;
+ goto out;
+ }
+ }
+
+ if (stk->channel.status == STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED
+ && stk->link_on_demand == TRUE) {
+ /*
+ * TODO
+ * activate the context, update the channel status
+ * once the context is activated, send the data immediately
+ * and flush the tx buffer
+ */
+ } else {
+ /*
+ * TODO
+ * send the data immediately, flush the tx buffer
+ */
+ rsp.send_data.tx_avail = stk->tx_buffer.data.len;
+ }
+
+out:
+ if (stk_respond(stk, &rsp, stk_command_cb))
+ stk_command_cb(&failure, stk);
+}
+
+static void stk_receive_data(struct ofono_stk *stk, unsigned char data_len)
+{
+ struct stk_response rsp;
+ struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
+
+ memset(&rsp, 0, sizeof(rsp));
+ rsp.result.type = STK_RESULT_TYPE_SUCCESS;
+
+ if (stk->rx_buffer.rx_remaining == 0) {
+ rsp.result.type = STK_RESULT_TYPE_MISSING_INFO;
+ goto out;
+ }
+
+ if (data_len > stk->rx_buffer.rx_remaining) {
+ rsp.result.type = STK_RESULT_TYPE_MISSING_INFO;
+ data_len = stk->rx_buffer.rx_remaining;
+ }
+
+ if (data_len > CHANNEL_DATA_OBJECT_MAX_LENGTH)
+ data_len = CHANNEL_DATA_OBJECT_MAX_LENGTH;
+
+ rsp.receive_data.rx_data.array = stk->rx_buffer.data.array;
+ rsp.receive_data.rx_data.len = data_len;
+ stk->rx_buffer.rx_remaining -= data_len;
+ rsp.receive_data.rx_remaining = stk->rx_buffer.rx_remaining;
+
+out:
+ if (stk_respond(stk, &rsp, stk_command_cb))
+ stk_command_cb(&failure, stk);
+
+ if (rsp.receive_data.rx_data.len && stk->rx_buffer.rx_remaining > 0)
+ memmove(stk->rx_buffer.data.array, stk->rx_buffer.data.array +
+ data_len, stk->rx_buffer.rx_remaining);
+}
+
static int duration_to_msecs(const struct stk_duration *duration)
{
int msecs = duration->interval;
@@ -2589,6 +2774,220 @@ static gboolean handle_command_launch_browser(const struct stk_command *cmd,
return FALSE;
}
+
+static void open_channel_cancel(struct ofono_stk *stk)
+{
+ /* TODO */
+}
+
+static void confirm_open_channel_cb(enum stk_agent_result result,
+ gboolean confirm,
+ void *user_data)
+{
+ struct ofono_stk *stk = user_data;
+
+ switch (result) {
+ case STK_AGENT_RESULT_TIMEOUT:
+ confirm = FALSE;
+ /* Fall through */
+
+ case STK_AGENT_RESULT_OK:
+ if (confirm)
+ break;
+
+ send_simple_response(stk, STK_RESULT_TYPE_USER_REJECT);
+ return;
+
+ case STK_AGENT_RESULT_TERMINATE:
+ default:
+ send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
+ return;
+ }
+
+ stk_open_channel(stk);
+}
+
+static gboolean handle_command_open_channel(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk)
+{
+ struct ofono_modem *modem = __ofono_atom_get_modem(stk->atom);
+ const struct stk_command_open_channel *oc = &cmd->open_channel;
+ struct ofono_atom *gprs_atom;
+ int err;
+
+ /* Check first if channel is available */
+ if (stk->channel.id) {
+ unsigned char addnl_info[1];
+
+ addnl_info[0] = STK_RESULT_ADDNL_BIP_PB_NO_CHANNEL_AVAIL;
+ ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
+ addnl_info);
+ return TRUE;
+ }
+
+ gprs_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_GPRS);
+ if (gprs_atom == NULL || !__ofono_atom_get_registered(gprs_atom)) {
+ rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ return TRUE;
+ }
+
+ stk->gprs = __ofono_atom_get_data(gprs_atom);
+ stk->respond_on_exit = TRUE;
+ stk->cancel_cmd = open_channel_cancel;
+
+ /*
+ * Don't ask for user confirmation if AID is a null data object
+ * or is not provided
+ */
+ if (oc->alpha_id && strlen(oc->alpha_id) > 0) {
+ char *alpha_id;
+
+ alpha_id = dbus_apply_text_attributes(oc->alpha_id,
+ &oc->text_attr);
+ if (alpha_id == NULL) {
+ rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+ return TRUE;
+ }
+
+ err = stk_agent_confirm_open_channel(stk->current_agent,
+ alpha_id,
+ &oc->icon_id,
+ confirm_open_channel_cb,
+ stk, NULL,
+ stk->timeout * 1000);
+ g_free(alpha_id);
+
+ if (err < 0) {
+ unsigned char no_cause_result[] = { 0x00 };
+
+ ADD_ERROR_RESULT(rsp->result,
+ STK_RESULT_TYPE_TERMINAL_BUSY,
+ no_cause_result);
+ return TRUE;
+ }
+
+ return FALSE;
+ }
+
+ stk_open_channel(stk);
+
+ return FALSE;
+}
+
+static gboolean handle_command_close_channel(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk)
+{
+ const struct stk_command_close_channel *cc = &cmd->close_channel;
+
+ /* Check if channel identifier is valid */
+ if (cmd->dst != (stk->channel.id | 0x20)) {
+ unsigned char addnl_info[1];
+
+ addnl_info[1] = STK_RESULT_ADDNL_BIP_PB_CHANNEL_ID_NOT_VALID;
+ ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
+ addnl_info);
+ return TRUE;
+ }
+
+ /*
+ * Don't inform the user about the link closing phase if AID is
+ * a null data object or is not provided
+ */
+ if (cc->alpha_id && strlen(cc->alpha_id) > 0)
+ stk_alpha_id_set(stk, cc->alpha_id, &cc->text_attr,
+ &cc->icon_id);
+
+ stk->respond_on_exit = TRUE;
+ stk->cancel_cmd = stk_request_cancel;
+
+ stk_close_channel(stk);
+
+ return FALSE;
+}
+
+static gboolean handle_command_receive_data(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk)
+{
+ const struct stk_command_receive_data *rd = &cmd->receive_data;
+
+ /* Check if channel identifier is valid or already closed */
+ if (cmd->dst != (stk->channel.id | 0x20)) {
+ unsigned char addnl_info[1];
+
+ addnl_info[1] = STK_RESULT_ADDNL_BIP_PB_CHANNEL_ID_NOT_VALID;
+ ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
+ addnl_info);
+ return TRUE;
+ }
+
+ /*
+ * Don't inform the user during data transfer if AID is
+ * a null data object or is not provided
+ */
+ if (rd->alpha_id && strlen(rd->alpha_id) > 0)
+ stk_alpha_id_set(stk, rd->alpha_id, &rd->text_attr,
+ &rd->icon_id);
+
+ stk->respond_on_exit = TRUE;
+ stk->cancel_cmd = stk_request_cancel;
+
+ stk_receive_data(stk, rd->data_len);
+
+ return FALSE;
+}
+
+static gboolean handle_command_send_data(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk)
+{
+ const struct stk_command_send_data *sd = &cmd->send_data;
+ unsigned char addnl_info[1];
+
+ /* Check if channel identifier is valid or already closed */
+ if (cmd->dst != (stk->channel.id | 0x20)) {
+ addnl_info[1] = STK_RESULT_ADDNL_BIP_PB_CHANNEL_ID_NOT_VALID;
+ ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
+ addnl_info);
+ return TRUE;
+ }
+
+ /* Check if the link was dropped */
+ if (stk->channel.status == STK_CHANNEL_LINK_DROPPED) {
+ addnl_info[1] = STK_RESULT_ADDNL_BIP_PB_CHANNEL_CLOSED;
+ ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
+ addnl_info);
+ return TRUE;
+ }
+
+ /*
+ * Don't inform the user during data transfer if AID is
+ * a null data object or is not provided
+ */
+ if (sd->alpha_id && strlen(sd->alpha_id) > 0)
+ stk_alpha_id_set(stk, sd->alpha_id, &sd->text_attr,
+ &sd->icon_id);
+
+ stk->respond_on_exit = TRUE;
+ stk->cancel_cmd = stk_request_cancel;
+
+ stk_send_data(stk, sd->data, cmd->qualifier);
+
+ return FALSE;
+}
+
+static gboolean handle_command_get_channel_status(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk)
+{
+ rsp->result.type = STK_RESULT_TYPE_SUCCESS;
+ rsp->channel_status.channel.id = stk->channel.id;
+ rsp->channel_status.channel.status = stk->channel.status;
+ return TRUE;
+}
+
static void stk_proactive_command_cancel(struct ofono_stk *stk)
{
if (stk->immediate_response)
@@ -2782,6 +3181,31 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
&rsp, stk);
break;
+ case STK_COMMAND_TYPE_OPEN_CHANNEL:
+ respond = handle_command_open_channel(stk->pending_cmd,
+ &rsp, stk);
+ break;
+
+ case STK_COMMAND_TYPE_CLOSE_CHANNEL:
+ respond = handle_command_close_channel(stk->pending_cmd,
+ &rsp, stk);
+ break;
+
+ case STK_COMMAND_TYPE_RECEIVE_DATA:
+ respond = handle_command_receive_data(stk->pending_cmd,
+ &rsp, stk);
+ break;
+
+ case STK_COMMAND_TYPE_SEND_DATA:
+ respond = handle_command_send_data(stk->pending_cmd,
+ &rsp, stk);
+ break;
+
+ case STK_COMMAND_TYPE_GET_CHANNEL_STATUS:
+ respond = handle_command_get_channel_status(stk->pending_cmd,
+ &rsp, stk);
+ break;
+
default:
rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
break;
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/8] stk: Introduce BIP command handlers
2011-05-20 16:26 ` [PATCH v4 2/8] stk: Introduce BIP command handlers Philippe Nunes
@ 2011-06-02 0:28 ` Denis Kenzior
2011-06-10 14:03 ` Philippe Nunes
0 siblings, 1 reply; 17+ messages in thread
From: Denis Kenzior @ 2011-06-02 0:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 15840 bytes --]
Hi Philippe,
On 05/20/2011 11:26 AM, Philippe Nunes wrote:
> ---
> src/stk.c | 426 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 425 insertions(+), 1 deletions(-)
>
This patch no longer applies. Please rebase and resubmit. Also, can I
ask you to break this patch up into several, e.g. one for Open Channel,
one for Close Channel, etc. This will make it much easier for me to review.
> diff --git a/src/stk.c b/src/stk.c
> index 8214b65..fb376a6 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -41,6 +41,7 @@
> #include "smsutil.h"
> #include "stkutil.h"
> #include "stkagent.h"
> +#include "gprs.h"
> #include "util.h"
>
> static GSList *g_drivers = NULL;
> @@ -79,6 +80,11 @@ struct ofono_stk {
>
> __ofono_sms_sim_download_cb_t sms_pp_cb;
> void *sms_pp_userdata;
> + struct stk_channel channel;
> + struct stk_channel_data rx_buffer;
> + struct stk_channel_data tx_buffer;
Are you sure you need these buffers? At least the RX buffer can
probably be ignored since you can always use the kernel socket buffer
instead. Copying from the kernel buffer into your own buffer just to
copy it into SIM's buffer seems like busywork.
The TX buffer is arguable, you might want to use non-blocking IO, but in
that case I would use a ring_buffer from GAtChat.
> + gboolean link_on_demand;
> + struct ofono_gprs *gprs;
> };
>
> struct envelope_op {
> @@ -104,6 +110,13 @@ static void timers_update(struct ofono_stk *stk);
> result.additional_len = sizeof(addn_info); \
> result.additional = addn_info; \
>
> +/*
> + * According the structure and coding of the Terminal response defined in
> + * TS 102 223 section 6.8, the maximum number of bytes possible for the channel
> + * data object is 243
> + */
> +#define CHANNEL_DATA_OBJECT_MAX_LENGTH 243
> +
> static int stk_respond(struct ofono_stk *stk, struct stk_response *rsp,
> ofono_stk_generic_cb_t cb)
> {
> @@ -474,12 +487,43 @@ static void emit_menu_changed(struct ofono_stk *stk)
> g_dbus_send_message(conn, signal);
> }
>
> +static void stk_close_channel(struct ofono_stk *stk)
> +{
> + /*
> + * TODO
> + * Deactivate and remove PDP context
> + * Send the Terminal Response once the PDP context is deactivated
> + */
> +
> + /* Temporary implementation */
> + g_free(stk->rx_buffer.data.array);
> + g_free(stk->tx_buffer.data.array);
> + stk->rx_buffer.data.array = NULL;
> + stk->tx_buffer.data.array = NULL;
> +
> + stk->channel.id = 0;
> + stk->channel.status = STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED;
> +
> + if (stk->pending_cmd &&
Would this function ever be called if pending_cmd is NULL?
> + stk->pending_cmd->type ==
> + STK_COMMAND_TYPE_CLOSE_CHANNEL)
> + send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
> +}
> +
> static void user_termination_cb(enum stk_agent_result result, void *user_data)
> {
> struct ofono_stk *stk = user_data;
>
> - if (result == STK_AGENT_RESULT_TERMINATE)
> + if (result != STK_AGENT_RESULT_TERMINATE)
> + return;
> +
> + if (stk->pending_cmd) {
> + stk->cancel_cmd(stk);
You can't actually do this since the agent does not support calling
stk_agent_request_cancel from within a callback.
> send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
> + }
> +
> + if (stk->channel.id)
> + stk_close_channel(stk);
> }
>
> static void stk_alpha_id_set(struct ofono_stk *stk,
> @@ -510,6 +554,147 @@ static void stk_alpha_id_unset(struct ofono_stk *stk)
> stk_agent_request_cancel(stk->current_agent);
> }
>
> +
> +static void stk_open_channel(struct ofono_stk *stk)
> +{
> + const struct stk_command_open_channel *oc;
> + struct stk_response rsp;
> + struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
> +
> + if (stk->pending_cmd == NULL ||
> + stk->pending_cmd->type != STK_COMMAND_TYPE_OPEN_CHANNEL)
doc/coding-style.txt item M4
> + return;
> +
> + oc = &stk->pending_cmd->open_channel;
> +
> + memset(&rsp, 0, sizeof(rsp));
> + rsp.result.type = STK_RESULT_TYPE_SUCCESS;
> +
> + stk->rx_buffer.data.array = g_try_malloc(oc->buf_size);
> + if (stk->rx_buffer.data.array == NULL) {
> + unsigned char no_cause_result[] = { 0x00 };
> +
> + ADD_ERROR_RESULT(rsp.result, STK_RESULT_TYPE_TERMINAL_BUSY,
> + no_cause_result);
> + goto out;
> + }
> +
> + stk->tx_buffer.data.array = g_try_malloc(oc->buf_size);
> + if (stk->tx_buffer.data.array == NULL) {
> + unsigned char no_cause_result[] = { 0x00 };
> +
> + ADD_ERROR_RESULT(rsp.result, STK_RESULT_TYPE_TERMINAL_BUSY,
> + no_cause_result);
> + goto out;
> + }
> +
> + stk->rx_buffer.data.len = oc->buf_size;
> + stk->rx_buffer.rx_remaining = 0;
> + stk->tx_buffer.data.len = oc->buf_size;
> + stk->tx_buffer.tx_avail = oc->buf_size;
> + stk->link_on_demand = (stk->pending_cmd->qualifier &
> + STK_OPEN_CHANNEL_FLAG_IMMEDIATE) ? FALSE : TRUE;
> +
> + /*
> + * TODO
> + * Add a new primary PDP context based on the provided settings
> + * Send the Terminal Response or wait until the PDP context is activated
> + * in case of immediate link establishment not in background.
> + */
> +out:
> + if (stk_respond(stk, &rsp, stk_command_cb))
> + stk_command_cb(&failure, stk);
> +
> + if (rsp.result.type == STK_RESULT_TYPE_NOT_CAPABLE && stk->channel.id)
> + stk_close_channel(stk);
> +}
> +
> +static void stk_send_data(struct ofono_stk *stk,
> + struct stk_common_byte_array data,
> + unsigned char qualifier)
> +{
> + struct stk_response rsp;
> + struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
> + unsigned int offset;
> +
> + memset(&rsp, 0, sizeof(rsp));
> + rsp.result.type = STK_RESULT_TYPE_SUCCESS;
> +
> + if (data.len > stk->tx_buffer.data.len ||
> + data.len > stk->tx_buffer.tx_avail) {
doc/coding-style.txt item M4
> + rsp.result.type = STK_RESULT_TYPE_BIP_ERROR;
> + goto out;
> + }
> +
> + if (data.len) {
> + offset = stk->tx_buffer.data.len - stk->tx_buffer.tx_avail;
> + memcpy(stk->tx_buffer.data.array + offset, data.array,
> + data.len);
> +
> + stk->tx_buffer.tx_avail -= data.len;
> +
> + if (qualifier == STK_SEND_DATA_STORE_DATA) {
> + rsp.send_data.tx_avail = stk->tx_buffer.tx_avail;
> + goto out;
> + }
> + }
> +
> + if (stk->channel.status == STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED
> + && stk->link_on_demand == TRUE) {
> + /*
> + * TODO
> + * activate the context, update the channel status
> + * once the context is activated, send the data immediately
> + * and flush the tx buffer
> + */
> + } else {
> + /*
> + * TODO
> + * send the data immediately, flush the tx buffer
> + */
> + rsp.send_data.tx_avail = stk->tx_buffer.data.len;
> + }
> +
> +out:
> + if (stk_respond(stk, &rsp, stk_command_cb))
> + stk_command_cb(&failure, stk);
> +}
> +
> +static void stk_receive_data(struct ofono_stk *stk, unsigned char data_len)
> +{
> + struct stk_response rsp;
> + struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
> +
> + memset(&rsp, 0, sizeof(rsp));
> + rsp.result.type = STK_RESULT_TYPE_SUCCESS;
> +
> + if (stk->rx_buffer.rx_remaining == 0) {
> + rsp.result.type = STK_RESULT_TYPE_MISSING_INFO;
> + goto out;
> + }
> +
> + if (data_len > stk->rx_buffer.rx_remaining) {
> + rsp.result.type = STK_RESULT_TYPE_MISSING_INFO;
> + data_len = stk->rx_buffer.rx_remaining;
> + }
> +
> + if (data_len > CHANNEL_DATA_OBJECT_MAX_LENGTH)
> + data_len = CHANNEL_DATA_OBJECT_MAX_LENGTH;
> +
> + rsp.receive_data.rx_data.array = stk->rx_buffer.data.array;
> + rsp.receive_data.rx_data.len = data_len;
> + stk->rx_buffer.rx_remaining -= data_len;
> + rsp.receive_data.rx_remaining = stk->rx_buffer.rx_remaining;
> +
> +out:
> + if (stk_respond(stk, &rsp, stk_command_cb))
> + stk_command_cb(&failure, stk);
> +
> + if (rsp.receive_data.rx_data.len && stk->rx_buffer.rx_remaining > 0)
> + memmove(stk->rx_buffer.data.array, stk->rx_buffer.data.array +
> + data_len, stk->rx_buffer.rx_remaining);
> +}
> +
> static int duration_to_msecs(const struct stk_duration *duration)
> {
> int msecs = duration->interval;
> @@ -2589,6 +2774,220 @@ static gboolean handle_command_launch_browser(const struct stk_command *cmd,
> return FALSE;
> }
>
> +
> +static void open_channel_cancel(struct ofono_stk *stk)
> +{
> + /* TODO */
> +}
> +
> +static void confirm_open_channel_cb(enum stk_agent_result result,
> + gboolean confirm,
> + void *user_data)
> +{
> + struct ofono_stk *stk = user_data;
> +
> + switch (result) {
> + case STK_AGENT_RESULT_TIMEOUT:
> + confirm = FALSE;
> + /* Fall through */
> +
> + case STK_AGENT_RESULT_OK:
> + if (confirm)
> + break;
> +
> + send_simple_response(stk, STK_RESULT_TYPE_USER_REJECT);
> + return;
> +
> + case STK_AGENT_RESULT_TERMINATE:
> + default:
> + send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED);
> + return;
> + }
> +
> + stk_open_channel(stk);
> +}
> +
> +static gboolean handle_command_open_channel(const struct stk_command *cmd,
> + struct stk_response *rsp,
> + struct ofono_stk *stk)
> +{
> + struct ofono_modem *modem = __ofono_atom_get_modem(stk->atom);
> + const struct stk_command_open_channel *oc = &cmd->open_channel;
> + struct ofono_atom *gprs_atom;
> + int err;
> +
> + /* Check first if channel is available */
> + if (stk->channel.id) {
> + unsigned char addnl_info[1];
> +
> + addnl_info[0] = STK_RESULT_ADDNL_BIP_PB_NO_CHANNEL_AVAIL;
> + ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
> + addnl_info);
> + return TRUE;
> + }
> +
> + gprs_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_GPRS);
> + if (gprs_atom == NULL || !__ofono_atom_get_registered(gprs_atom)) {
> + rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
> + return TRUE;
> + }
> +
> + stk->gprs = __ofono_atom_get_data(gprs_atom);
> + stk->respond_on_exit = TRUE;
> + stk->cancel_cmd = open_channel_cancel;
> +
> + /*
> + * Don't ask for user confirmation if AID is a null data object
> + * or is not provided
> + */
> + if (oc->alpha_id && strlen(oc->alpha_id) > 0) {
Calling strlen here is a bit overkill, you can always use
oc->alpha_id[0] != '\0'
> + char *alpha_id;
> +
> + alpha_id = dbus_apply_text_attributes(oc->alpha_id,
> + &oc->text_attr);
> + if (alpha_id == NULL) {
> + rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
> + return TRUE;
> + }
> +
> + err = stk_agent_confirm_open_channel(stk->current_agent,
> + alpha_id,
> + &oc->icon_id,
> + confirm_open_channel_cb,
> + stk, NULL,
> + stk->timeout * 1000);
> + g_free(alpha_id);
> +
> + if (err < 0) {
> + unsigned char no_cause_result[] = { 0x00 };
> +
> + ADD_ERROR_RESULT(rsp->result,
> + STK_RESULT_TYPE_TERMINAL_BUSY,
> + no_cause_result);
> + return TRUE;
> + }
> +
> + return FALSE;
> + }
> +
> + stk_open_channel(stk);
> +
> + return FALSE;
> +}
> +
> +static gboolean handle_command_close_channel(const struct stk_command *cmd,
> + struct stk_response *rsp,
> + struct ofono_stk *stk)
> +{
> + const struct stk_command_close_channel *cc = &cmd->close_channel;
> +
> + /* Check if channel identifier is valid */
> + if (cmd->dst != (stk->channel.id | 0x20)) {
> + unsigned char addnl_info[1];
> +
> + addnl_info[1] = STK_RESULT_ADDNL_BIP_PB_CHANNEL_ID_NOT_VALID;
> + ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
> + addnl_info);
> + return TRUE;
> + }
> +
> + /*
> + * Don't inform the user about the link closing phase if AID is
> + * a null data object or is not provided
> + */
> + if (cc->alpha_id && strlen(cc->alpha_id) > 0)
> + stk_alpha_id_set(stk, cc->alpha_id, &cc->text_attr,
> + &cc->icon_id);
> +
> + stk->respond_on_exit = TRUE;
> + stk->cancel_cmd = stk_request_cancel;
> +
> + stk_close_channel(stk);
> +
> + return FALSE;
> +}
> +
> +static gboolean handle_command_receive_data(const struct stk_command *cmd,
> + struct stk_response *rsp,
> + struct ofono_stk *stk)
> +{
> + const struct stk_command_receive_data *rd = &cmd->receive_data;
> +
> + /* Check if channel identifier is valid or already closed */
> + if (cmd->dst != (stk->channel.id | 0x20)) {
> + unsigned char addnl_info[1];
> +
> + addnl_info[1] = STK_RESULT_ADDNL_BIP_PB_CHANNEL_ID_NOT_VALID;
> + ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
> + addnl_info);
> + return TRUE;
> + }
> +
> + /*
> + * Don't inform the user during data transfer if AID is
> + * a null data object or is not provided
> + */
> + if (rd->alpha_id && strlen(rd->alpha_id) > 0)
> + stk_alpha_id_set(stk, rd->alpha_id, &rd->text_attr,
> + &rd->icon_id);
> +
> + stk->respond_on_exit = TRUE;
> + stk->cancel_cmd = stk_request_cancel;
> +
> + stk_receive_data(stk, rd->data_len);
> +
> + return FALSE;
> +}
> +
> +static gboolean handle_command_send_data(const struct stk_command *cmd,
> + struct stk_response *rsp,
> + struct ofono_stk *stk)
> +{
> + const struct stk_command_send_data *sd = &cmd->send_data;
> + unsigned char addnl_info[1];
> +
> + /* Check if channel identifier is valid or already closed */
> + if (cmd->dst != (stk->channel.id | 0x20)) {
> + addnl_info[1] = STK_RESULT_ADDNL_BIP_PB_CHANNEL_ID_NOT_VALID;
> + ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
> + addnl_info);
> + return TRUE;
> + }
> +
> + /* Check if the link was dropped */
> + if (stk->channel.status == STK_CHANNEL_LINK_DROPPED) {
> + addnl_info[1] = STK_RESULT_ADDNL_BIP_PB_CHANNEL_CLOSED;
> + ADD_ERROR_RESULT(rsp->result, STK_RESULT_TYPE_BIP_ERROR,
> + addnl_info);
> + return TRUE;
> + }
> +
> + /*
> + * Don't inform the user during data transfer if AID is
> + * a null data object or is not provided
> + */
> + if (sd->alpha_id && strlen(sd->alpha_id) > 0)
> + stk_alpha_id_set(stk, sd->alpha_id, &sd->text_attr,
> + &sd->icon_id);
> +
> + stk->respond_on_exit = TRUE;
> + stk->cancel_cmd = stk_request_cancel;
> +
> + stk_send_data(stk, sd->data, cmd->qualifier);
> +
> + return FALSE;
> +}
> +
> +static gboolean handle_command_get_channel_status(const struct stk_command *cmd,
> + struct stk_response *rsp,
> + struct ofono_stk *stk)
> +{
> + rsp->result.type = STK_RESULT_TYPE_SUCCESS;
> + rsp->channel_status.channel.id = stk->channel.id;
> + rsp->channel_status.channel.status = stk->channel.status;
> + return TRUE;
> +}
> +
> static void stk_proactive_command_cancel(struct ofono_stk *stk)
> {
> if (stk->immediate_response)
> @@ -2782,6 +3181,31 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
> &rsp, stk);
> break;
>
> + case STK_COMMAND_TYPE_OPEN_CHANNEL:
> + respond = handle_command_open_channel(stk->pending_cmd,
> + &rsp, stk);
> + break;
> +
> + case STK_COMMAND_TYPE_CLOSE_CHANNEL:
> + respond = handle_command_close_channel(stk->pending_cmd,
> + &rsp, stk);
> + break;
> +
> + case STK_COMMAND_TYPE_RECEIVE_DATA:
> + respond = handle_command_receive_data(stk->pending_cmd,
> + &rsp, stk);
> + break;
> +
> + case STK_COMMAND_TYPE_SEND_DATA:
> + respond = handle_command_send_data(stk->pending_cmd,
> + &rsp, stk);
> + break;
> +
> + case STK_COMMAND_TYPE_GET_CHANNEL_STATUS:
> + respond = handle_command_get_channel_status(stk->pending_cmd,
> + &rsp, stk);
> + break;
> +
> default:
> rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
> break;
Regards,
-Denis
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/8] stk: Introduce BIP command handlers
2011-06-02 0:28 ` Denis Kenzior
@ 2011-06-10 14:03 ` Philippe Nunes
2011-06-08 8:01 ` Denis Kenzior
0 siblings, 1 reply; 17+ messages in thread
From: Philippe Nunes @ 2011-06-10 14:03 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4839 bytes --]
Hi Denis,
On 06/02/2011 02:28 AM, Denis Kenzior wrote:
> Hi Philippe,
>
> On 05/20/2011 11:26 AM, Philippe Nunes wrote:
>> ---
>> src/stk.c | 426 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 425 insertions(+), 1 deletions(-)
>>
>
> This patch no longer applies. Please rebase and resubmit. Also, can I
> ask you to break this patch up into several, e.g. one for Open Channel,
> one for Close Channel, etc. This will make it much easier for me to review.
>
yes, sure.
>> diff --git a/src/stk.c b/src/stk.c
>> index 8214b65..fb376a6 100644
>> --- a/src/stk.c
>> +++ b/src/stk.c
>> @@ -41,6 +41,7 @@
>> #include "smsutil.h"
>> #include "stkutil.h"
>> #include "stkagent.h"
>> +#include "gprs.h"
>> #include "util.h"
>>
>> static GSList *g_drivers = NULL;
>> @@ -79,6 +80,11 @@ struct ofono_stk {
>>
>> __ofono_sms_sim_download_cb_t sms_pp_cb;
>> void *sms_pp_userdata;
>> + struct stk_channel channel;
>> + struct stk_channel_data rx_buffer;
>> + struct stk_channel_data tx_buffer;
>
> Are you sure you need these buffers? At least the RX buffer can
> probably be ignored since you can always use the kernel socket buffer
> instead. Copying from the kernel buffer into your own buffer just to
> copy it into SIM's buffer seems like busywork.
>
The idea of the Rx buffer is to introduce an intermediate buffer since
it's up to the UICC to collect the received data with the proactive
command 'Receive data'. But before that, we need to read the kernel
buffer to determine the amount of data available and inform the UICC
about this size.
> The TX buffer is arguable, you might want to use non-blocking IO, but in
> that case I would use a ring_buffer from GAtChat.
For STK, what is the added value of a ring buffer? The mechanism for STK
is to store data in the TX buffer and once the PDU is complete, flush it
by sending the data. So we can't store before the Tx buffer is empty.
>
>> + gboolean link_on_demand;
>> + struct ofono_gprs *gprs;
>> };
>>
>> struct envelope_op {
>> @@ -104,6 +110,13 @@ static void timers_update(struct ofono_stk *stk);
>> result.additional_len = sizeof(addn_info); \
>> result.additional = addn_info; \
>>
>> +/*
>> + * According the structure and coding of the Terminal response defined in
>> + * TS 102 223 section 6.8, the maximum number of bytes possible for the channel
>> + * data object is 243
>> + */
>> +#define CHANNEL_DATA_OBJECT_MAX_LENGTH 243
>> +
>> static int stk_respond(struct ofono_stk *stk, struct stk_response *rsp,
>> ofono_stk_generic_cb_t cb)
>> {
>> @@ -474,12 +487,43 @@ static void emit_menu_changed(struct ofono_stk *stk)
>> g_dbus_send_message(conn, signal);
>> }
>>
>> +static void stk_close_channel(struct ofono_stk *stk)
>> +{
>> + /*
>> + * TODO
>> + * Deactivate and remove PDP context
>> + * Send the Terminal Response once the PDP context is deactivated
>> + */
>> +
>> + /* Temporary implementation */
>> + g_free(stk->rx_buffer.data.array);
>> + g_free(stk->tx_buffer.data.array);
>> + stk->rx_buffer.data.array = NULL;
>> + stk->tx_buffer.data.array = NULL;
>> +
>> + stk->channel.id = 0;
>> + stk->channel.status = STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED;
>> +
>> + if (stk->pending_cmd&&
>
> Would this function ever be called if pending_cmd is NULL?
stk_close_channel may be called from the callback user_termination_cb.
Therefore, the pending cmd is likely to be NULL once the Terminal
response is sent.
>
>> + stk->pending_cmd->type ==
>> + STK_COMMAND_TYPE_CLOSE_CHANNEL)
>> + send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
>> +}
>> +
>> static void user_termination_cb(enum stk_agent_result result, void *user_data)
>> {
>> struct ofono_stk *stk = user_data;
>>
>> - if (result == STK_AGENT_RESULT_TERMINATE)
>> + if (result != STK_AGENT_RESULT_TERMINATE)
>> + return;
>> +
>> + if (stk->pending_cmd) {
>> + stk->cancel_cmd(stk);
>
> You can't actually do this since the agent does not support calling
> stk_agent_request_cancel from within a callback.
OK, thank you for pointing this issue. But then I realize that I don't
need to expressly release the stk session agent (through
stk_agent_request_cancel). We just need to wait for the "End session"
notification which is likely to be received as a result of the Terminal
response "session terminated by user".
However I'm willing to keep the condition:
if (stk->channel.id)
stk_close_channel(stk);
instead of introducing additional cases in your new switch case since
for me, it's relevant to close the channel (if any) once the session is
terminated (whatever is the proactive command).
Regards,
Philippe.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 2/8] stk: Introduce BIP command handlers
2011-06-10 14:03 ` Philippe Nunes
@ 2011-06-08 8:01 ` Denis Kenzior
0 siblings, 0 replies; 17+ messages in thread
From: Denis Kenzior @ 2011-06-08 8:01 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2184 bytes --]
Hi Philippe,
>>> __ofono_sms_sim_download_cb_t sms_pp_cb;
>>> void *sms_pp_userdata;
>>> + struct stk_channel channel;
>>> + struct stk_channel_data rx_buffer;
>>> + struct stk_channel_data tx_buffer;
>>
>> Are you sure you need these buffers? At least the RX buffer can
>> probably be ignored since you can always use the kernel socket buffer
>> instead. Copying from the kernel buffer into your own buffer just to
>> copy it into SIM's buffer seems like busywork.
>>
>
> The idea of the Rx buffer is to introduce an intermediate buffer since
> it's up to the UICC to collect the received data with the proactive
> command 'Receive data'. But before that, we need to read the kernel
> buffer to determine the amount of data available and inform the UICC
> about this size.
>
If this is the only reason you're introducing an rx buffer, then please
don't. There are ioctls on tcp / udp sockets that give you this
information. Please see FIONREAD ioctl.
>
>> The TX buffer is arguable, you might want to use non-blocking IO, but in
>> that case I would use a ring_buffer from GAtChat.
>
> For STK, what is the added value of a ring buffer? The mechanism for STK
> is to store data in the TX buffer and once the PDU is complete, flush it
> by sending the data. So we can't store before the Tx buffer is empty.
>
>
Because you would have to use non-blocking IO, and your number of bytes
written to the kernel socket is not guaranteed to be the same as the
contents of the tx buffer. So you either must memmove or use a ring
buffer which is more efficient.
>>
>> You can't actually do this since the agent does not support calling
>> stk_agent_request_cancel from within a callback.
>
> OK, thank you for pointing this issue. But then I realize that I don't
> need to expressly release the stk session agent (through
> stk_agent_request_cancel). We just need to wait for the "End session"
> notification which is likely to be received as a result of the Terminal
> response "session terminated by user".
We've already had this discussion ;) Please don't rely on such behavior.
Regards,
-Denis
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 3/8] gprs: Add 'stk' gprs context type
2011-05-20 16:26 [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Philippe Nunes
2011-05-20 16:26 ` [PATCH v4 2/8] stk: Introduce BIP command handlers Philippe Nunes
@ 2011-05-20 16:26 ` Philippe Nunes
2011-06-01 5:30 ` Denis Kenzior
2011-05-20 16:26 ` [PATCH v4 4/8] gprs: Add private APIs for adding/activating/removing hidden PDP contexts Philippe Nunes
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Philippe Nunes @ 2011-05-20 16:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]
---
include/gprs-context.h | 1 +
src/gprs.c | 7 +++++++
2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/include/gprs-context.h b/include/gprs-context.h
index f82fcaa..e7976a1 100644
--- a/include/gprs-context.h
+++ b/include/gprs-context.h
@@ -46,6 +46,7 @@ enum ofono_gprs_context_type {
OFONO_GPRS_CONTEXT_TYPE_MMS,
OFONO_GPRS_CONTEXT_TYPE_WAP,
OFONO_GPRS_CONTEXT_TYPE_IMS,
+ OFONO_GPRS_CONTEXT_TYPE_STK,
};
struct ofono_gprs_primary_context {
diff --git a/src/gprs.c b/src/gprs.c
index deffeb8..9657a3e 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -188,6 +188,8 @@ static const char *gprs_context_default_name(enum ofono_gprs_context_type type)
return "WAP";
case OFONO_GPRS_CONTEXT_TYPE_IMS:
return "IMS";
+ case OFONO_GPRS_CONTEXT_TYPE_STK:
+ return "STK";
}
return NULL;
@@ -207,6 +209,8 @@ static const char *gprs_context_type_to_string(
return "wap";
case OFONO_GPRS_CONTEXT_TYPE_IMS:
return "ims";
+ case OFONO_GPRS_CONTEXT_TYPE_STK:
+ return "stk";
}
return NULL;
@@ -227,6 +231,9 @@ static gboolean gprs_context_string_to_type(const char *str,
} else if (g_str_equal(str, "ims")) {
*out = OFONO_GPRS_CONTEXT_TYPE_IMS;
return TRUE;
+ } else if (g_str_equal(str, "stk")) {
+ *out = OFONO_GPRS_CONTEXT_TYPE_STK;
+ return TRUE;
}
return FALSE;
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 3/8] gprs: Add 'stk' gprs context type
2011-05-20 16:26 ` [PATCH v4 3/8] gprs: Add 'stk' gprs context type Philippe Nunes
@ 2011-06-01 5:30 ` Denis Kenzior
0 siblings, 0 replies; 17+ messages in thread
From: Denis Kenzior @ 2011-06-01 5:30 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]
Hi Philippe,
On 05/20/2011 11:26 AM, Philippe Nunes wrote:
> ---
> include/gprs-context.h | 1 +
> src/gprs.c | 7 +++++++
> 2 files changed, 8 insertions(+), 0 deletions(-)
>
Please refer to 'Submitting patches' section in HACKING. We do prefer
these to be two separate patches.
> diff --git a/include/gprs-context.h b/include/gprs-context.h
> index f82fcaa..e7976a1 100644
> --- a/include/gprs-context.h
> +++ b/include/gprs-context.h
> @@ -46,6 +46,7 @@ enum ofono_gprs_context_type {
> OFONO_GPRS_CONTEXT_TYPE_MMS,
> OFONO_GPRS_CONTEXT_TYPE_WAP,
> OFONO_GPRS_CONTEXT_TYPE_IMS,
> + OFONO_GPRS_CONTEXT_TYPE_STK,
> };
>
> struct ofono_gprs_primary_context {
> diff --git a/src/gprs.c b/src/gprs.c
> index deffeb8..9657a3e 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -188,6 +188,8 @@ static const char *gprs_context_default_name(enum ofono_gprs_context_type type)
> return "WAP";
> case OFONO_GPRS_CONTEXT_TYPE_IMS:
> return "IMS";
> + case OFONO_GPRS_CONTEXT_TYPE_STK:
> + return "STK";
> }
>
> return NULL;
> @@ -207,6 +209,8 @@ static const char *gprs_context_type_to_string(
> return "wap";
> case OFONO_GPRS_CONTEXT_TYPE_IMS:
> return "ims";
> + case OFONO_GPRS_CONTEXT_TYPE_STK:
> + return "stk";
> }
>
> return NULL;
> @@ -227,6 +231,9 @@ static gboolean gprs_context_string_to_type(const char *str,
> } else if (g_str_equal(str, "ims")) {
> *out = OFONO_GPRS_CONTEXT_TYPE_IMS;
> return TRUE;
> + } else if (g_str_equal(str, "stk")) {
> + *out = OFONO_GPRS_CONTEXT_TYPE_STK;
> + return TRUE;
> }
>
> return FALSE;
I don't really see the point of these changes though. We do not want to
expose STK contexts over D-Bus, so none of these are going to be used.
Regards,
-Denis
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 4/8] gprs: Add private APIs for adding/activating/removing hidden PDP contexts
2011-05-20 16:26 [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Philippe Nunes
2011-05-20 16:26 ` [PATCH v4 2/8] stk: Introduce BIP command handlers Philippe Nunes
2011-05-20 16:26 ` [PATCH v4 3/8] gprs: Add 'stk' gprs context type Philippe Nunes
@ 2011-05-20 16:26 ` Philippe Nunes
2011-06-01 6:18 ` Denis Kenzior
2011-05-20 16:26 ` [PATCH v4 5/8] stk: Use Gprs private APIs to handle the Open channel/Close Channel Philippe Nunes
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Philippe Nunes @ 2011-05-20 16:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 8279 bytes --]
---
src/gprs.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/ofono.h | 24 ++++++
2 files changed, 263 insertions(+), 0 deletions(-)
diff --git a/src/gprs.c b/src/gprs.c
index 9657a3e..86d95bc 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -147,6 +147,8 @@ struct pri_context {
struct ofono_gprs_primary_context context;
struct ofono_gprs_context *context_driver;
struct ofono_gprs *gprs;
+ void *notify;
+ void *notify_data;
};
static void gprs_netreg_update(struct ofono_gprs *gprs);
@@ -356,6 +358,35 @@ static struct pri_context *gprs_context_by_path(struct ofono_gprs *gprs,
return NULL;
}
+static struct pri_context *gprs_context_by_id(struct ofono_gprs *gprs,
+ unsigned int id)
+{
+ GSList *l;
+
+ for (l = gprs->contexts; l; l = l->next) {
+ struct pri_context *ctx = l->data;
+
+ if (ctx->id == id)
+ return ctx;
+ }
+
+ return NULL;
+}
+
+static struct pri_context *gprs_get_default_context(struct ofono_gprs *gprs)
+{
+ GSList *l;
+
+ for (l = gprs->contexts; l; l = l->next) {
+ struct pri_context *ctx = l->data;
+
+ if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_INTERNET)
+ return ctx;
+ }
+
+ return NULL;
+}
+
static void context_settings_free(struct context_settings *settings)
{
if (settings->ipv4) {
@@ -537,6 +568,9 @@ static void signal_settings(struct pri_context *ctx, const char *prop,
DBusMessageIter iter;
struct context_settings *settings;
+ if (path == NULL)
+ return;
+
signal = dbus_message_new_signal(path,
OFONO_CONNECTION_CONTEXT_INTERFACE,
"PropertyChanged");
@@ -2968,3 +3002,208 @@ void *ofono_gprs_get_data(struct ofono_gprs *gprs)
{
return gprs->driver_data;
}
+
+unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
+ enum ofono_gprs_context_type context_type,
+ enum ofono_gprs_proto proto,
+ const char *apn,
+ const char *username,
+ const char *password,
+ const char *host)
+{
+ unsigned int id;
+ struct pri_context *context;
+ struct pri_context *default_ctx = NULL;
+ const char *name;
+
+ /* Sanity check */
+ if (apn && strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
+ return 0;
+
+ if (username && strlen(username) > OFONO_GPRS_MAX_USERNAME_LENGTH)
+ return 0;
+
+ if (password && strlen(password) > OFONO_GPRS_MAX_PASSWORD_LENGTH)
+ return 0;
+
+ if (apn == NULL || (username == NULL && password == NULL)) {
+ /* take the default primary internet context */
+ default_ctx = gprs_get_default_context(gprs);
+
+ if (default_ctx == NULL && apn == NULL)
+ return 0;
+ }
+
+ if (apn && is_valid_apn(apn) == FALSE)
+ return 0;
+
+ name = gprs_context_default_name(context_type);
+ if (name == NULL)
+ return 0;
+
+ if (gprs->last_context_id)
+ id = idmap_alloc_next(gprs->pid_map, gprs->last_context_id);
+ else
+ id = idmap_alloc(gprs->pid_map);
+
+ if (id > idmap_get_max(gprs->pid_map))
+ return 0;
+
+ context = pri_context_create(gprs, name, context_type);
+ if (context == NULL) {
+ idmap_put(gprs->pid_map, id);
+ return 0;
+ }
+
+ context->id = id;
+
+ if (username != NULL)
+ strcpy(context->context.username, username);
+
+ if (password != NULL)
+ strcpy(context->context.password, password);
+
+ if (username == NULL && password == NULL && default_ctx) {
+ if (default_ctx->context.username)
+ strcpy(context->context.username,
+ default_ctx->context.username);
+ if (default_ctx->context.password)
+ strcpy(context->context.password,
+ default_ctx->context.password);
+ }
+
+ if (apn)
+ strcpy(context->context.apn, apn);
+ else if (default_ctx) {
+ strcpy(context->context.apn, default_ctx->context.apn);
+ if (default_ctx->context.username)
+ strcpy(context->context.username,
+ default_ctx->context.username);
+ if (default_ctx->context.password)
+ strcpy(context->context.password,
+ default_ctx->context.password);
+ }
+
+ context->context.proto = proto;
+
+ gprs->last_context_id = id;
+ gprs->contexts = g_slist_append(gprs->contexts, context);
+
+ return id;
+}
+
+static void activate_request_callback(const struct ofono_error *error,
+ void *data)
+{
+ struct pri_context *ctx = data;
+ struct ofono_gprs_context *gc = ctx->context_driver;
+
+ DBG("%p", ctx);
+
+ if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+ DBG("Activating context failed with error: %s",
+ telephony_error_to_str(error));
+ context_settings_free(ctx->context_driver->settings);
+ release_context(ctx);
+
+ if (ctx->notify)
+ ((__ofono_gprs_add_pdp_context_cb_t) ctx->notify)
+ (-ENOSYS, NULL, NULL, ctx->notify_data);
+ return;
+ }
+
+ ctx->active = TRUE;
+
+ if (gc->settings->interface != NULL) {
+ pri_ifupdown(gc->settings->interface, TRUE);
+
+ if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_STK &&
+ gc->settings->ipv4) {
+ pri_set_ipv4_addr(gc->settings->interface,
+ gc->settings->ipv4->ip);
+ }
+ }
+
+ if (ctx->notify && gc->settings->ipv4)
+ ((__ofono_gprs_add_pdp_context_cb_t) ctx->notify)
+ (0, gc->settings->interface,
+ gc->settings->ipv4->ip,
+ ctx->notify_data);
+}
+
+int __ofono_gprs_activate_pdp_context(struct ofono_gprs *gprs, unsigned int id,
+ __ofono_gprs_add_pdp_context_cb_t cb,
+ void *data)
+{
+ struct pri_context *ctx;
+ struct ofono_gprs_context *gc;
+
+ ctx = gprs_context_by_id(gprs, id);
+ if (ctx == NULL)
+ return -EINVAL;
+
+ if (ctx->active == TRUE)
+ return 0;
+
+ if (assign_context(ctx) == FALSE)
+ return -ENOSYS;
+
+ gc = ctx->context_driver;
+
+ ctx->notify = cb;
+ ctx->notify_data = data;
+
+ gc->driver->activate_primary(gc, &ctx->context,
+ activate_request_callback, ctx);
+ return 0;
+}
+
+static void remove_request_callback(const struct ofono_error *error,
+ void *data)
+{
+ struct pri_context *ctx = data;
+ struct ofono_gprs *gprs = ctx->gprs;
+ int err = 0;
+
+ if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+ DBG("Removing context failed with error: %s",
+ telephony_error_to_str(error));
+ err = -ENOSYS;
+ }
+
+ pri_reset_context_settings(ctx);
+ release_context(ctx);
+ idmap_put(gprs->pid_map, ctx->id);
+ gprs->contexts = g_slist_remove(gprs->contexts, ctx);
+
+ if (ctx->notify)
+ ((__ofono_gprs_remove_pdp_context_cb_t) ctx->notify)
+ (err, ctx->notify_data);
+}
+
+int __ofono_gprs_remove_pdp_context(struct ofono_gprs *gprs, unsigned int id,
+ __ofono_gprs_remove_pdp_context_cb_t cb,
+ void *data)
+{
+ struct pri_context *ctx;
+
+ ctx = gprs_context_by_id(gprs, id);
+ if (ctx == NULL)
+ return -EINVAL;
+
+ if (ctx->active) {
+ struct ofono_gprs_context *gc = ctx->context_driver;
+
+ ctx->notify = cb;
+ ctx->notify_data = data;
+
+ gc->driver->deactivate_primary(gc, ctx->context.cid,
+ remove_request_callback, ctx);
+ return -EINPROGRESS;
+ }
+
+ idmap_put(gprs->pid_map, ctx->id);
+ gprs->contexts = g_slist_remove(gprs->contexts, ctx);
+
+ return 0;
+}
diff --git a/src/ofono.h b/src/ofono.h
index 82d7e34..ee5864d 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -239,6 +239,30 @@ gboolean __ofono_call_settings_is_busy(struct ofono_call_settings *cs);
#include <ofono/phonebook.h>
#include <ofono/gprs.h>
#include <ofono/gprs-context.h>
+
+typedef void (*__ofono_gprs_add_pdp_context_cb_t)(int error,
+ const char *interface,
+ const char *ip,
+ void *data);
+
+typedef void (*__ofono_gprs_remove_pdp_context_cb_t)(int error, void *data);
+
+unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
+ enum ofono_gprs_context_type type,
+ enum ofono_gprs_proto proto,
+ const char *apn,
+ const char *username,
+ const char *password,
+ const char *host);
+
+int __ofono_gprs_remove_pdp_context(struct ofono_gprs *gprs, unsigned int id,
+ __ofono_gprs_remove_pdp_context_cb_t cb,
+ void *data);
+
+int __ofono_gprs_activate_pdp_context(struct ofono_gprs *gprs, unsigned int id,
+ __ofono_gprs_add_pdp_context_cb_t cb,
+ void *data);
+
#include <ofono/radio-settings.h>
#include <ofono/audio-settings.h>
#include <ofono/ctm.h>
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 4/8] gprs: Add private APIs for adding/activating/removing hidden PDP contexts
2011-05-20 16:26 ` [PATCH v4 4/8] gprs: Add private APIs for adding/activating/removing hidden PDP contexts Philippe Nunes
@ 2011-06-01 6:18 ` Denis Kenzior
2011-06-10 9:44 ` Philippe Nunes
0 siblings, 1 reply; 17+ messages in thread
From: Denis Kenzior @ 2011-06-01 6:18 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 10168 bytes --]
Hi Philippe,
On 05/20/2011 11:26 AM, Philippe Nunes wrote:
> ---
> src/gprs.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/ofono.h | 24 ++++++
> 2 files changed, 263 insertions(+), 0 deletions(-)
>
> diff --git a/src/gprs.c b/src/gprs.c
> index 9657a3e..86d95bc 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -147,6 +147,8 @@ struct pri_context {
> struct ofono_gprs_primary_context context;
> struct ofono_gprs_context *context_driver;
> struct ofono_gprs *gprs;
> + void *notify;
> + void *notify_data;
> };
>
Modifying pri_context for this does not seem like a good idea, I suggest
you make your own structure since pretty much all the members of this
structure are not relevant to what you're trying to accomplish.
As a general philosophy we prefer clearer code (even if there's more of
it) than trying to complicate the logic of code designed for a
completely different purpose.
> static void gprs_netreg_update(struct ofono_gprs *gprs);
> @@ -356,6 +358,35 @@ static struct pri_context *gprs_context_by_path(struct ofono_gprs *gprs,
> return NULL;
> }
>
> +static struct pri_context *gprs_context_by_id(struct ofono_gprs *gprs,
> + unsigned int id)
> +{
> + GSList *l;
> +
> + for (l = gprs->contexts; l; l = l->next) {
> + struct pri_context *ctx = l->data;
> +
> + if (ctx->id == id)
> + return ctx;
> + }
> +
> + return NULL;
> +}
> +
> +static struct pri_context *gprs_get_default_context(struct ofono_gprs *gprs)
> +{
> + GSList *l;
> +
> + for (l = gprs->contexts; l; l = l->next) {
> + struct pri_context *ctx = l->data;
> +
> + if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_INTERNET)
> + return ctx;
> + }
> +
> + return NULL;
> +}
> +
> static void context_settings_free(struct context_settings *settings)
> {
> if (settings->ipv4) {
> @@ -537,6 +568,9 @@ static void signal_settings(struct pri_context *ctx, const char *prop,
> DBusMessageIter iter;
> struct context_settings *settings;
>
> + if (path == NULL)
> + return;
> +
> signal = dbus_message_new_signal(path,
> OFONO_CONNECTION_CONTEXT_INTERFACE,
> "PropertyChanged");
> @@ -2968,3 +3002,208 @@ void *ofono_gprs_get_data(struct ofono_gprs *gprs)
> {
> return gprs->driver_data;
> }
> +
> +unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
> + enum ofono_gprs_context_type context_type,
> + enum ofono_gprs_proto proto,
> + const char *apn,
> + const char *username,
> + const char *password,
> + const char *host)
What is the purpose of the host parameter? Doesn't STK use IP addresses
for the destination?
> +{
Returning int here would be better:
> + unsigned int id;
> + struct pri_context *context;
> + struct pri_context *default_ctx = NULL;
> + const char *name;
> +
> + /* Sanity check */
> + if (apn && strlen(apn) > OFONO_GPRS_MAX_APN_LENGTH)
> + return 0;
Then you can return meaningful error here, e.g. -EINVAL
> +
> + if (username && strlen(username) > OFONO_GPRS_MAX_USERNAME_LENGTH)
> + return 0;
> +
> + if (password && strlen(password) > OFONO_GPRS_MAX_PASSWORD_LENGTH)
> + return 0;
> +
> + if (apn == NULL || (username == NULL && password == NULL)) {
> + /* take the default primary internet context */
> + default_ctx = gprs_get_default_context(gprs);
> +
> + if (default_ctx == NULL && apn == NULL)
> + return 0;
Maybe ENOENT here
> + }
> +
> + if (apn && is_valid_apn(apn) == FALSE)
> + return 0;
> +
etc
This will allow you to return meaningful terminal responses in case
things fail.
> + name = gprs_context_default_name(context_type);
> + if (name == NULL)
> + return 0;
> +
> + if (gprs->last_context_id)
> + id = idmap_alloc_next(gprs->pid_map, gprs->last_context_id);
> + else
> + id = idmap_alloc(gprs->pid_map);
> +
> + if (id > idmap_get_max(gprs->pid_map))
> + return 0;
> +
> + context = pri_context_create(gprs, name, context_type);
> + if (context == NULL) {
> + idmap_put(gprs->pid_map, id);
> + return 0;
> + }
> +
As mentioned previously, I don't see the point of creating pri_context
structure for this. So you can rip much of this stuff out.
> + context->id = id;
> +
> + if (username != NULL)
> + strcpy(context->context.username, username);
> +
> + if (password != NULL)
> + strcpy(context->context.password, password);
> +
> + if (username == NULL && password == NULL && default_ctx) {
> + if (default_ctx->context.username)
> + strcpy(context->context.username,
> + default_ctx->context.username);
> + if (default_ctx->context.password)
> + strcpy(context->context.password,
> + default_ctx->context.password);
> + }
> +
> + if (apn)
> + strcpy(context->context.apn, apn);
> + else if (default_ctx) {
> + strcpy(context->context.apn, default_ctx->context.apn);
> + if (default_ctx->context.username)
> + strcpy(context->context.username,
> + default_ctx->context.username);
> + if (default_ctx->context.password)
> + strcpy(context->context.password,
> + default_ctx->context.password);
> + }
> +
> + context->context.proto = proto;
> +
> + gprs->last_context_id = id;
> + gprs->contexts = g_slist_append(gprs->contexts, context);
> +
> + return id;
> +}
> +
> +static void activate_request_callback(const struct ofono_error *error,
> + void *data)
> +{
> + struct pri_context *ctx = data;
> + struct ofono_gprs_context *gc = ctx->context_driver;
> +
> + DBG("%p", ctx);
> +
> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> + DBG("Activating context failed with error: %s",
> + telephony_error_to_str(error));
> + context_settings_free(ctx->context_driver->settings);
> + release_context(ctx);
> +
> + if (ctx->notify)
> + ((__ofono_gprs_add_pdp_context_cb_t) ctx->notify)
> + (-ENOSYS, NULL, NULL, ctx->notify_data);
> + return;
> + }
> +
> + ctx->active = TRUE;
> +
> + if (gc->settings->interface != NULL) {
> + pri_ifupdown(gc->settings->interface, TRUE);
> +
> + if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_STK &&
> + gc->settings->ipv4) {
> + pri_set_ipv4_addr(gc->settings->interface,
> + gc->settings->ipv4->ip);
> + }
> + }
> +
> + if (ctx->notify && gc->settings->ipv4)
> + ((__ofono_gprs_add_pdp_context_cb_t) ctx->notify)
> + (0, gc->settings->interface,
> + gc->settings->ipv4->ip,
> + ctx->notify_data);
> +}
> +
> +int __ofono_gprs_activate_pdp_context(struct ofono_gprs *gprs, unsigned int id,
> + __ofono_gprs_add_pdp_context_cb_t cb,
> + void *data)
> +{
> + struct pri_context *ctx;
> + struct ofono_gprs_context *gc;
> +
> + ctx = gprs_context_by_id(gprs, id);
> + if (ctx == NULL)
> + return -EINVAL;
> +
> + if (ctx->active == TRUE)
> + return 0;
> +
> + if (assign_context(ctx) == FALSE)
> + return -ENOSYS;
> +
> + gc = ctx->context_driver;
> +
> + ctx->notify = cb;
> + ctx->notify_data = data;
> +
> + gc->driver->activate_primary(gc, &ctx->context,
> + activate_request_callback, ctx);
> + return 0;
> +}
> +
> +static void remove_request_callback(const struct ofono_error *error,
> + void *data)
> +{
> + struct pri_context *ctx = data;
> + struct ofono_gprs *gprs = ctx->gprs;
> + int err = 0;
> +
> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> + DBG("Removing context failed with error: %s",
> + telephony_error_to_str(error));
> + err = -ENOSYS;
> + }
> +
> + pri_reset_context_settings(ctx);
> + release_context(ctx);
> + idmap_put(gprs->pid_map, ctx->id);
> + gprs->contexts = g_slist_remove(gprs->contexts, ctx);
> +
> + if (ctx->notify)
> + ((__ofono_gprs_remove_pdp_context_cb_t) ctx->notify)
> + (err, ctx->notify_data);
> +}
> +
> +int __ofono_gprs_remove_pdp_context(struct ofono_gprs *gprs, unsigned int id,
> + __ofono_gprs_remove_pdp_context_cb_t cb,
> + void *data)
> +{
> + struct pri_context *ctx;
> +
> + ctx = gprs_context_by_id(gprs, id);
> + if (ctx == NULL)
> + return -EINVAL;
> +
> + if (ctx->active) {
> + struct ofono_gprs_context *gc = ctx->context_driver;
> +
> + ctx->notify = cb;
> + ctx->notify_data = data;
> +
> + gc->driver->deactivate_primary(gc, ctx->context.cid,
> + remove_request_callback, ctx);
> + return -EINPROGRESS;
> + }
> +
> + idmap_put(gprs->pid_map, ctx->id);
> + gprs->contexts = g_slist_remove(gprs->contexts, ctx);
> +
> + return 0;
> +}
> diff --git a/src/ofono.h b/src/ofono.h
> index 82d7e34..ee5864d 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -239,6 +239,30 @@ gboolean __ofono_call_settings_is_busy(struct ofono_call_settings *cs);
> #include <ofono/phonebook.h>
> #include <ofono/gprs.h>
> #include <ofono/gprs-context.h>
> +
> +typedef void (*__ofono_gprs_add_pdp_context_cb_t)(int error,
> + const char *interface,
> + const char *ip,
> + void *data);
> +
> +typedef void (*__ofono_gprs_remove_pdp_context_cb_t)(int error, void *data);
> +
> +unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
> + enum ofono_gprs_context_type type,
> + enum ofono_gprs_proto proto,
> + const char *apn,
> + const char *username,
> + const char *password,
> + const char *host);
> +
I don't think this is needed at all. All STK needs to do is just say
'please activate a context with these params'. You can pretty much get
away with __ofono_gprs_activate_context and __ofono_gprs_deactivate_context.
> +int __ofono_gprs_remove_pdp_context(struct ofono_gprs *gprs, unsigned int id,
> + __ofono_gprs_remove_pdp_context_cb_t cb,
> + void *data);
> +
Are you sure you really need the callback function here? The spec is a
little fuzzy on when the terminal response is actually sent.
> +int __ofono_gprs_activate_pdp_context(struct ofono_gprs *gprs, unsigned int id,
> + __ofono_gprs_add_pdp_context_cb_t cb,
> + void *data);
> +
> #include <ofono/radio-settings.h>
> #include <ofono/audio-settings.h>
> #include <ofono/ctm.h>
Regards,
-Denis
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 4/8] gprs: Add private APIs for adding/activating/removing hidden PDP contexts
2011-06-01 6:18 ` Denis Kenzior
@ 2011-06-10 9:44 ` Philippe Nunes
2011-06-08 7:46 ` Denis Kenzior
0 siblings, 1 reply; 17+ messages in thread
From: Philippe Nunes @ 2011-06-10 9:44 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 12419 bytes --]
Hi Denis,
On 06/01/2011 08:18 AM, Denis Kenzior wrote:
> Hi Philippe,
>
> On 05/20/2011 11:26 AM, Philippe Nunes wrote:
>> ---
>> src/gprs.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> src/ofono.h | 24 ++++++
>> 2 files changed, 263 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/gprs.c b/src/gprs.c
>> index 9657a3e..86d95bc 100644
>> --- a/src/gprs.c
>> +++ b/src/gprs.c
>> @@ -147,6 +147,8 @@ struct pri_context {
>> struct ofono_gprs_primary_context context;
>> struct ofono_gprs_context *context_driver;
>> struct ofono_gprs *gprs;
>> + void *notify;
>> + void *notify_data;
>> };
>>
>
> Modifying pri_context for this does not seem like a good idea, I suggest
> you make your own structure since pretty much all the members of this
> structure are not relevant to what you're trying to accomplish.
>
> As a general philosophy we prefer clearer code (even if there's more of
> it) than trying to complicate the logic of code designed for a
> completely different purpose.
>
Ok, I can remove this callback pointer since this callback is only
needed for the private primary context created by the STK atom.
>> static void gprs_netreg_update(struct ofono_gprs *gprs);
>> @@ -356,6 +358,35 @@ static struct pri_context *gprs_context_by_path(struct ofono_gprs *gprs,
>> return NULL;
>> }
>>
>> +static struct pri_context *gprs_context_by_id(struct ofono_gprs *gprs,
>> + unsigned int id)
>> +{
>> + GSList *l;
>> +
>> + for (l = gprs->contexts; l; l = l->next) {
>> + struct pri_context *ctx = l->data;
>> +
>> + if (ctx->id == id)
>> + return ctx;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static struct pri_context *gprs_get_default_context(struct ofono_gprs *gprs)
>> +{
>> + GSList *l;
>> +
>> + for (l = gprs->contexts; l; l = l->next) {
>> + struct pri_context *ctx = l->data;
>> +
>> + if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_INTERNET)
>> + return ctx;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> static void context_settings_free(struct context_settings *settings)
>> {
>> if (settings->ipv4) {
>> @@ -537,6 +568,9 @@ static void signal_settings(struct pri_context *ctx, const char *prop,
>> DBusMessageIter iter;
>> struct context_settings *settings;
>>
>> + if (path == NULL)
>> + return;
>> +
>> signal = dbus_message_new_signal(path,
>> OFONO_CONNECTION_CONTEXT_INTERFACE,
>> "PropertyChanged");
>> @@ -2968,3 +3002,208 @@ void *ofono_gprs_get_data(struct ofono_gprs *gprs)
>> {
>> return gprs->driver_data;
>> }
>> +
>> +unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
>> + enum ofono_gprs_context_type context_type,
>> + enum ofono_gprs_proto proto,
>> + const char *apn,
>> + const char *username,
>> + const char *password,
>> + const char *host)
>
> What is the purpose of the host parameter? Doesn't STK use IP addresses
> for the destination?
>
Here, the purpose of the host parameter is to set the interface with a
special host route (id the IP stream with this host/destination address
has to go through this interface).
Indeed, I need to bind the UDP/TCP socket to the interface created
precisely for STK purpose.
But, I can remove this parameter since you suggested to make
pri_setproxy (gprs.c) into a utility function and call it from stk.c
>> +{
>
> Returning int here would be better:
>
>> + unsigned int id;
>> + struct pri_context *context;
>> + struct pri_context *default_ctx = NULL;
>> + const char *name;
>> +
>> + /* Sanity check */
>> + if (apn&& strlen(apn)> OFONO_GPRS_MAX_APN_LENGTH)
>> + return 0;
>
> Then you can return meaningful error here, e.g. -EINVAL
>
>> +
>> + if (username&& strlen(username)> OFONO_GPRS_MAX_USERNAME_LENGTH)
>> + return 0;
>> +
>> + if (password&& strlen(password)> OFONO_GPRS_MAX_PASSWORD_LENGTH)
>> + return 0;
>> +
>> + if (apn == NULL || (username == NULL&& password == NULL)) {
>> + /* take the default primary internet context */
>> + default_ctx = gprs_get_default_context(gprs);
>> +
>> + if (default_ctx == NULL&& apn == NULL)
>> + return 0;
>
> Maybe ENOENT here
>
>> + }
>> +
>> + if (apn&& is_valid_apn(apn) == FALSE)
>> + return 0;
>> +
>
> etc
>
> This will allow you to return meaningful terminal responses in case
> things fail.
>
>> + name = gprs_context_default_name(context_type);
>> + if (name == NULL)
>> + return 0;
>> +
>> + if (gprs->last_context_id)
>> + id = idmap_alloc_next(gprs->pid_map, gprs->last_context_id);
>> + else
>> + id = idmap_alloc(gprs->pid_map);
>> +
>> + if (id> idmap_get_max(gprs->pid_map))
>> + return 0;
>> +
>> + context = pri_context_create(gprs, name, context_type);
>> + if (context == NULL) {
>> + idmap_put(gprs->pid_map, id);
>> + return 0;
>> + }
>> +
>
> As mentioned previously, I don't see the point of creating pri_context
> structure for this. So you can rip much of this stuff out.
I don't understand why you think we don't need to create a pri_context.
Could you clarify please?
Even if no parameters are provided by the UICC (id use default
parameters), I still need to setup a dedicated primary context with
defaults parameters since we can't double-activate an existing primary
context.
>
>> + context->id = id;
>> +
>> + if (username != NULL)
>> + strcpy(context->context.username, username);
>> +
>> + if (password != NULL)
>> + strcpy(context->context.password, password);
>> +
>> + if (username == NULL&& password == NULL&& default_ctx) {
>> + if (default_ctx->context.username)
>> + strcpy(context->context.username,
>> + default_ctx->context.username);
>> + if (default_ctx->context.password)
>> + strcpy(context->context.password,
>> + default_ctx->context.password);
>> + }
>> +
>> + if (apn)
>> + strcpy(context->context.apn, apn);
>> + else if (default_ctx) {
>> + strcpy(context->context.apn, default_ctx->context.apn);
>> + if (default_ctx->context.username)
>> + strcpy(context->context.username,
>> + default_ctx->context.username);
>> + if (default_ctx->context.password)
>> + strcpy(context->context.password,
>> + default_ctx->context.password);
>> + }
>> +
>> + context->context.proto = proto;
>> +
>> + gprs->last_context_id = id;
>> + gprs->contexts = g_slist_append(gprs->contexts, context);
>> +
>> + return id;
>> +}
>> +
>> +static void activate_request_callback(const struct ofono_error *error,
>> + void *data)
>> +{
>> + struct pri_context *ctx = data;
>> + struct ofono_gprs_context *gc = ctx->context_driver;
>> +
>> + DBG("%p", ctx);
>> +
>> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>> + DBG("Activating context failed with error: %s",
>> + telephony_error_to_str(error));
>> + context_settings_free(ctx->context_driver->settings);
>> + release_context(ctx);
>> +
>> + if (ctx->notify)
>> + ((__ofono_gprs_add_pdp_context_cb_t) ctx->notify)
>> + (-ENOSYS, NULL, NULL, ctx->notify_data);
>> + return;
>> + }
>> +
>> + ctx->active = TRUE;
>> +
>> + if (gc->settings->interface != NULL) {
>> + pri_ifupdown(gc->settings->interface, TRUE);
>> +
>> + if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_STK&&
>> + gc->settings->ipv4) {
>> + pri_set_ipv4_addr(gc->settings->interface,
>> + gc->settings->ipv4->ip);
>> + }
>> + }
>> +
>> + if (ctx->notify&& gc->settings->ipv4)
>> + ((__ofono_gprs_add_pdp_context_cb_t) ctx->notify)
>> + (0, gc->settings->interface,
>> + gc->settings->ipv4->ip,
>> + ctx->notify_data);
>> +}
>> +
>> +int __ofono_gprs_activate_pdp_context(struct ofono_gprs *gprs, unsigned int id,
>> + __ofono_gprs_add_pdp_context_cb_t cb,
>> + void *data)
>> +{
>> + struct pri_context *ctx;
>> + struct ofono_gprs_context *gc;
>> +
>> + ctx = gprs_context_by_id(gprs, id);
>> + if (ctx == NULL)
>> + return -EINVAL;
>> +
>> + if (ctx->active == TRUE)
>> + return 0;
>> +
>> + if (assign_context(ctx) == FALSE)
>> + return -ENOSYS;
>> +
>> + gc = ctx->context_driver;
>> +
>> + ctx->notify = cb;
>> + ctx->notify_data = data;
>> +
>> + gc->driver->activate_primary(gc,&ctx->context,
>> + activate_request_callback, ctx);
>> + return 0;
>> +}
>> +
>> +static void remove_request_callback(const struct ofono_error *error,
>> + void *data)
>> +{
>> + struct pri_context *ctx = data;
>> + struct ofono_gprs *gprs = ctx->gprs;
>> + int err = 0;
>> +
>> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>> + DBG("Removing context failed with error: %s",
>> + telephony_error_to_str(error));
>> + err = -ENOSYS;
>> + }
>> +
>> + pri_reset_context_settings(ctx);
>> + release_context(ctx);
>> + idmap_put(gprs->pid_map, ctx->id);
>> + gprs->contexts = g_slist_remove(gprs->contexts, ctx);
>> +
>> + if (ctx->notify)
>> + ((__ofono_gprs_remove_pdp_context_cb_t) ctx->notify)
>> + (err, ctx->notify_data);
>> +}
>> +
>> +int __ofono_gprs_remove_pdp_context(struct ofono_gprs *gprs, unsigned int id,
>> + __ofono_gprs_remove_pdp_context_cb_t cb,
>> + void *data)
>> +{
>> + struct pri_context *ctx;
>> +
>> + ctx = gprs_context_by_id(gprs, id);
>> + if (ctx == NULL)
>> + return -EINVAL;
>> +
>> + if (ctx->active) {
>> + struct ofono_gprs_context *gc = ctx->context_driver;
>> +
>> + ctx->notify = cb;
>> + ctx->notify_data = data;
>> +
>> + gc->driver->deactivate_primary(gc, ctx->context.cid,
>> + remove_request_callback, ctx);
>> + return -EINPROGRESS;
>> + }
>> +
>> + idmap_put(gprs->pid_map, ctx->id);
>> + gprs->contexts = g_slist_remove(gprs->contexts, ctx);
>> +
>> + return 0;
>> +}
>> diff --git a/src/ofono.h b/src/ofono.h
>> index 82d7e34..ee5864d 100644
>> --- a/src/ofono.h
>> +++ b/src/ofono.h
>> @@ -239,6 +239,30 @@ gboolean __ofono_call_settings_is_busy(struct ofono_call_settings *cs);
>> #include<ofono/phonebook.h>
>> #include<ofono/gprs.h>
>> #include<ofono/gprs-context.h>
>> +
>> +typedef void (*__ofono_gprs_add_pdp_context_cb_t)(int error,
>> + const char *interface,
>> + const char *ip,
>> + void *data);
>> +
>> +typedef void (*__ofono_gprs_remove_pdp_context_cb_t)(int error, void *data);
>> +
>> +unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
>> + enum ofono_gprs_context_type type,
>> + enum ofono_gprs_proto proto,
>> + const char *apn,
>> + const char *username,
>> + const char *password,
>> + const char *host);
>> +
>
> I don't think this is needed at all. All STK needs to do is just say
> 'please activate a context with these params'. You can pretty much get
> away with __ofono_gprs_activate_context and __ofono_gprs_deactivate_context.
>
When "on link demand" mode is requested, we just need to setup the PDP
context and return the PDP identifier. The PDP context activation is
done only once we receive a proactive command SEND DATA with the
qualifier "Send data immediately".
So, I don't see how to differentiate this mode without the API
"__ofono_gprs_add_pdp_context".
>> +int __ofono_gprs_remove_pdp_context(struct ofono_gprs *gprs, unsigned int id,
>> + __ofono_gprs_remove_pdp_context_cb_t cb,
>> + void *data);
>> +
>
> Are you sure you really need the callback function here? The spec is a
> little fuzzy on when the terminal response is actually sent.
At first sight, yes, that was also my opinion but then I saw the
examples in annex I (TS 102 223).
For Close channel, we can see the following sequence:
UICC Terminal SGSN
|---------------------> | |
| Close Channel | |
| | --------------------------> |
| | Deactivate PDP context request|
| | <-----------------------------|
| | Deactivate PDP context accept |
|<----------------------
Terminal response
That's why, I introduced the callback to trigger the Terminal response.
Regards,
Philippe.
>
>> +int __ofono_gprs_activate_pdp_context(struct ofono_gprs *gprs, unsigned int id,
>> + __ofono_gprs_add_pdp_context_cb_t cb,
>> + void *data);
>> +
>> #include<ofono/radio-settings.h>
>> #include<ofono/audio-settings.h>
>> #include<ofono/ctm.h>
>
> Regards,
> -Denis
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v4 4/8] gprs: Add private APIs for adding/activating/removing hidden PDP contexts
2011-06-10 9:44 ` Philippe Nunes
@ 2011-06-08 7:46 ` Denis Kenzior
0 siblings, 0 replies; 17+ messages in thread
From: Denis Kenzior @ 2011-06-08 7:46 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3348 bytes --]
Hi Philippe,
>>
>> As mentioned previously, I don't see the point of creating pri_context
>> structure for this. So you can rip much of this stuff out.
>
>
> I don't understand why you think we don't need to create a pri_context.
> Could you clarify please?
>
> Even if no parameters are provided by the UICC (id use default
> parameters), I still need to setup a dedicated primary context with
> defaults parameters since we can't double-activate an existing primary
> context.
>
struct pri_context is used to store information pertinent to the D-Bus
API. Namely all the context attributes that are exposed on the
ConnectionContext interface. You are not using any of them, and we do
not want to ever expose STK contexts over D-Bus.
In effect you're (ab)using a structure that was never meant for your
particular purpose, and moreover not using 90% of the members of that
structure.
Whenever you are faced with a situation like this you need to ask
yourself whether making a new structure would make more sense,
particularly from ease of implementation and code readability.
Generally the answer is 'yes'.
<snip>
>>> +unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
>>> + enum ofono_gprs_context_type type,
>>> + enum ofono_gprs_proto proto,
>>> + const char *apn,
>>> + const char *username,
>>> + const char *password,
>>> + const char *host);
>>> +
>>
>> I don't think this is needed at all. All STK needs to do is just say
>> 'please activate a context with these params'. You can pretty much get
>> away with __ofono_gprs_activate_context and
>> __ofono_gprs_deactivate_context.
>>
>
> When "on link demand" mode is requested, we just need to setup the PDP
> context and return the PDP identifier. The PDP context activation is
> done only once we receive a proactive command SEND DATA with the
> qualifier "Send data immediately".
> So, I don't see how to differentiate this mode without the API
> "__ofono_gprs_add_pdp_context".
>
There's nothing stopping you from recording the PDP context parameters
in stk.c and providing them to activate_context when some data is
actually sent.
>
>>> +int __ofono_gprs_remove_pdp_context(struct ofono_gprs *gprs,
>>> unsigned int id,
>>> + __ofono_gprs_remove_pdp_context_cb_t cb,
>>> + void *data);
>>> +
>>
>> Are you sure you really need the callback function here? The spec is a
>> little fuzzy on when the terminal response is actually sent.
>
>
> At first sight, yes, that was also my opinion but then I saw the
> examples in annex I (TS 102 223).
>
> For Close channel, we can see the following sequence:
>
> UICC Terminal SGSN
>
> |---------------------> | |
> | Close Channel | |
> | | --------------------------> |
> | | Deactivate PDP context request|
> | | <-----------------------------|
> | | Deactivate PDP context accept |
> |<----------------------
> Terminal response
>
> That's why, I introduced the callback to trigger the Terminal response.
>
Fair enough.
Regards,
-Denis
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 5/8] stk: Use Gprs private APIs to handle the Open channel/Close Channel
2011-05-20 16:26 [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Philippe Nunes
` (2 preceding siblings ...)
2011-05-20 16:26 ` [PATCH v4 4/8] gprs: Add private APIs for adding/activating/removing hidden PDP contexts Philippe Nunes
@ 2011-05-20 16:26 ` Philippe Nunes
2011-05-20 16:26 ` [PATCH v4 6/8] gprs: Add a host route for STK context type Philippe Nunes
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Philippe Nunes @ 2011-05-20 16:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 7975 bytes --]
---
src/stk.c | 198 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 175 insertions(+), 23 deletions(-)
diff --git a/src/stk.c b/src/stk.c
index fb376a6..d9f1e25 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -28,6 +28,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
+#include <arpa/inet.h>
#include <glib.h>
#include <gdbus.h>
@@ -85,6 +86,7 @@ struct ofono_stk {
struct stk_channel_data tx_buffer;
gboolean link_on_demand;
struct ofono_gprs *gprs;
+ struct stk_bearer_description bearer_desc;
};
struct envelope_op {
@@ -487,27 +489,57 @@ static void emit_menu_changed(struct ofono_stk *stk)
g_dbus_send_message(conn, signal);
}
-static void stk_close_channel(struct ofono_stk *stk)
+static void ofono_stk_remove_pdp_context_cb(int error, void *data)
{
- /*
- * TODO
- * Deactivate and remove PDP context
- * Send the Terminal Response once the PDP context is deactivated
- */
+ struct ofono_stk *stk = data;
- /* Temporary implementation */
+ DBG("");
+
+ stk->channel.status = STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED;
+
+ if (stk->pending_cmd && stk->pending_cmd->type ==
+ STK_COMMAND_TYPE_CLOSE_CHANNEL) {
+ if (error < 0)
+ send_simple_response(stk, STK_RESULT_TYPE_NOT_CAPABLE);
+ else
+ send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
+ } else {
+ /*
+ * TODO
+ * Send Event channel status
+ */
+ }
+
+ /* free the buffers even in case of error */
g_free(stk->rx_buffer.data.array);
g_free(stk->tx_buffer.data.array);
stk->rx_buffer.data.array = NULL;
stk->tx_buffer.data.array = NULL;
stk->channel.id = 0;
- stk->channel.status = STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED;
+}
+
+static void stk_close_channel(struct ofono_stk *stk)
+{
+ int err;
+
+ err = __ofono_gprs_remove_pdp_context(stk->gprs, stk->channel.id,
+ ofono_stk_remove_pdp_context_cb, stk);
- if (stk->pending_cmd &&
- stk->pending_cmd->type ==
+ if (err == 0) {
+ g_free(stk->rx_buffer.data.array);
+ g_free(stk->tx_buffer.data.array);
+ stk->rx_buffer.data.array = NULL;
+ stk->tx_buffer.data.array = NULL;
+
+ stk->channel.id = 0;
+ stk->channel.status =
+ STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED;
+
+ if (stk->pending_cmd && stk->pending_cmd->type ==
STK_COMMAND_TYPE_CLOSE_CHANNEL)
- send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
+ send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
+ }
}
static void user_termination_cb(enum stk_agent_result result, void *user_data)
@@ -554,12 +586,66 @@ static void stk_alpha_id_unset(struct ofono_stk *stk)
stk_agent_request_cancel(stk->current_agent);
}
+static void ofono_stk_activate_pdp_context_cb(int error, const char *interface,
+ const char *ip,
+ void *data)
+{
+ struct ofono_stk *stk = data;
+ struct stk_response rsp;
+ struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
+
+ DBG("");
+
+ memset(&rsp, 0, sizeof(rsp));
+
+ if (error < 0) {
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ } else {
+ DBG("Interface %s, IP = %s", interface, ip);
+ stk->channel.status = STK_CHANNEL_PACKET_DATA_SERVICE_ACTIVATED;
+ }
+
+ if (stk->pending_cmd == NULL) {
+ /*
+ * TODO
+ * Send Event channel status
+ */
+ return;
+ }
+
+ if (stk->pending_cmd->type == STK_COMMAND_TYPE_OPEN_CHANNEL) {
+ rsp.open_channel.channel.id = stk->channel.id;
+ rsp.open_channel.channel.status = stk->channel.status;
+ rsp.open_channel.buf_size = stk->rx_buffer.data.len;
+ memcpy(&rsp.open_channel.bearer_desc, &stk->bearer_desc,
+ sizeof(struct stk_bearer_description));
+ } else if (stk->pending_cmd->type == STK_COMMAND_TYPE_SEND_DATA &&
+ stk->link_on_demand) {
+ /*
+ * TODO
+ * send the data immediately, flush the tx buffer
+ */
+
+ rsp.send_data.tx_avail = stk->tx_buffer.tx_avail;
+ }
+
+out:
+ if (stk_respond(stk, &rsp, stk_command_cb))
+ stk_command_cb(&failure, stk);
+
+ if (error < 0)
+ stk_close_channel(stk);
+}
static void stk_open_channel(struct ofono_stk *stk)
{
const struct stk_command_open_channel *oc;
struct stk_response rsp;
struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
+ char *host = NULL;
+ unsigned int id;
+ int err;
if (stk->pending_cmd == NULL ||
stk->pending_cmd->type != STK_COMMAND_TYPE_OPEN_CHANNEL)
@@ -570,6 +656,37 @@ static void stk_open_channel(struct ofono_stk *stk)
memset(&rsp, 0, sizeof(rsp));
rsp.result.type = STK_RESULT_TYPE_SUCCESS;
+ if (oc->data_dest_addr.type == STK_ADDRESS_IPV4) {
+ struct in_addr addr;
+
+ host = g_try_malloc0(INET_ADDRSTRLEN);
+ memset(host, 0, sizeof(host));
+ addr.s_addr = oc->data_dest_addr.addr.ipv4;
+ inet_ntop(AF_INET, &addr, host, INET_ADDRSTRLEN);
+ } else {
+ /*
+ * For now, only the bearer type "GPRS / UTRAN packet service /
+ * E-UTRAN" is supported.
+ * For such bearer, according 3GPP TS 31.111, the packet data
+ * protocol type is IP, so only IPv4 addresses are considered.
+ */
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+
+ id = __ofono_gprs_add_pdp_context(stk->gprs,
+ OFONO_GPRS_CONTEXT_TYPE_STK,
+ OFONO_GPRS_PROTO_IP, oc->apn,
+ oc->text_usr, oc->text_passwd, host);
+
+ if (id == 0) {
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+
+ stk->channel.id = id;
+ ofono_info("Channel %d", id);
+
stk->rx_buffer.data.array = g_try_malloc(oc->buf_size);
if (stk->rx_buffer.data.array == NULL) {
unsigned char no_cause_result[] = { 0x00 };
@@ -592,16 +709,44 @@ static void stk_open_channel(struct ofono_stk *stk)
stk->rx_buffer.rx_remaining = 0;
stk->tx_buffer.data.len = oc->buf_size;
stk->tx_buffer.tx_avail = oc->buf_size;
+
+ memcpy(&stk->bearer_desc, &oc->bearer_desc,
+ sizeof(struct stk_bearer_description));
stk->link_on_demand = (stk->pending_cmd->qualifier &
STK_OPEN_CHANNEL_FLAG_IMMEDIATE) ? FALSE : TRUE;
- /*
- * TODO
- * Add a new primary PDP context based on the provided settings
- * Send the Terminal Response or wait until the PDP context is activated
- * in case of immediate link establishment not in background.
- */
+ if (stk->link_on_demand) {
+ rsp.open_channel.channel.id = id;
+ rsp.open_channel.channel.status =
+ STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED;
+ rsp.open_channel.buf_size = oc->buf_size;
+ goto out;
+ }
+
+ err = __ofono_gprs_activate_pdp_context(stk->gprs, id,
+ ofono_stk_activate_pdp_context_cb, stk);
+
+ if (err < 0) {
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+
+ /* In background mode, send the terminal response now */
+ if (stk->pending_cmd->qualifier & STK_OPEN_CHANNEL_FLAG_BACKGROUND) {
+ rsp.open_channel.channel.id = id;
+ rsp.open_channel.channel.status =
+ STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED;
+ rsp.open_channel.buf_size = oc->buf_size;
+ goto out;
+ }
+
+ g_free(host);
+ /* wait for the PDP context activation to send the Terminal Response */
+ return;
+
out:
+ g_free(host);
+
if (stk_respond(stk, &rsp, stk_command_cb))
stk_command_cb(&failure, stk);
@@ -641,12 +786,19 @@ static void stk_send_data(struct ofono_stk *stk,
if (stk->channel.status == STK_CHANNEL_PACKET_DATA_SERVICE_NOT_ACTIVATED
&& stk->link_on_demand == TRUE) {
- /*
- * TODO
- * activate the context, update the channel status
- * once the context is activated, send the data immediately
- * and flush the tx buffer
- */
+ int err;
+
+ err = __ofono_gprs_activate_pdp_context(stk->gprs,
+ stk->channel.id,
+ ofono_stk_activate_pdp_context_cb,
+ stk);
+
+ if (err < 0) {
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+
+ return;
} else {
/*
* TODO
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 6/8] gprs: Add a host route for STK context type
2011-05-20 16:26 [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Philippe Nunes
` (3 preceding siblings ...)
2011-05-20 16:26 ` [PATCH v4 5/8] stk: Use Gprs private APIs to handle the Open channel/Close Channel Philippe Nunes
@ 2011-05-20 16:26 ` Philippe Nunes
2011-06-01 6:26 ` Denis Kenzior
2011-05-20 16:26 ` [PATCH v4 7/8] stk: Add support of the Setup event list proactive command Philippe Nunes
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Philippe Nunes @ 2011-05-20 16:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4112 bytes --]
---
src/gprs.c | 45 ++++++++++++++++++++++++++-------------------
1 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/src/gprs.c b/src/gprs.c
index 86d95bc..a867ea1 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -141,8 +141,8 @@ struct pri_context {
unsigned int id;
char *path;
char *key;
- char *proxy_host;
- uint16_t proxy_port;
+ char *host;
+ uint16_t port;
DBusMessage *pending;
struct ofono_gprs_primary_context context;
struct ofono_gprs_context *context_driver;
@@ -616,16 +616,16 @@ static void pri_parse_proxy(struct pri_context *ctx, const char *proxy)
host += 3;
if (strcasecmp(scheme, "https") == 0)
- ctx->proxy_port = 443;
+ ctx->port = 443;
else if (strcasecmp(scheme, "http") == 0)
- ctx->proxy_port = 80;
+ ctx->port = 80;
else {
g_free(scheme);
return;
}
} else {
host = scheme;
- ctx->proxy_port = 80;
+ ctx->port = 80;
}
path = strchr(host, '/');
@@ -639,12 +639,12 @@ static void pri_parse_proxy(struct pri_context *ctx, const char *proxy)
if (*end == '\0') {
*port = '\0';
- ctx->proxy_port = tmp;
+ ctx->port = tmp;
}
}
- g_free(ctx->proxy_host);
- ctx->proxy_host = g_strdup(host);
+ g_free(ctx->host);
+ ctx->host = g_strdup(host);
g_free(scheme);
}
@@ -728,7 +728,7 @@ done:
close(sk);
}
-static void pri_setproxy(const char *interface, const char *proxy)
+static void pri_add_host_route(const char *interface, const char *host)
{
struct rtentry rt;
struct sockaddr_in addr;
@@ -747,7 +747,7 @@ static void pri_setproxy(const char *interface, const char *proxy)
memset(&addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
- addr.sin_addr.s_addr = inet_addr(proxy);
+ addr.sin_addr.s_addr = inet_addr(host);
memcpy(&rt.rt_dst, &addr, sizeof(rt.rt_dst));
memset(&addr, 0, sizeof(addr));
@@ -761,7 +761,7 @@ static void pri_setproxy(const char *interface, const char *proxy)
memcpy(&rt.rt_genmask, &addr, sizeof(rt.rt_genmask));
if (ioctl(sk, SIOCADDRT, &rt) < 0)
- ofono_error("Failed to add proxy host route");
+ ofono_error("Failed to add host route");
close(sk);
}
@@ -788,12 +788,13 @@ static void pri_reset_context_settings(struct pri_context *ctx)
pri_context_signal_settings(ctx, signal_ipv4, signal_ipv6);
- if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_MMS) {
+ if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_MMS ||
+ ctx->type == OFONO_GPRS_CONTEXT_TYPE_STK) {
pri_set_ipv4_addr(interface, NULL);
- g_free(ctx->proxy_host);
- ctx->proxy_host = NULL;
- ctx->proxy_port = 0;
+ g_free(ctx->host);
+ ctx->host = NULL;
+ ctx->port = 0;
}
pri_ifupdown(interface, FALSE);
@@ -811,12 +812,12 @@ static void pri_update_mms_context_settings(struct pri_context *ctx)
pri_parse_proxy(ctx, ctx->message_proxy);
- DBG("proxy %s port %u", ctx->proxy_host, ctx->proxy_port);
+ DBG("host %s port %u", ctx->host, ctx->port);
pri_set_ipv4_addr(settings->interface, settings->ipv4->ip);
- if (ctx->proxy_host)
- pri_setproxy(settings->interface, ctx->proxy_host);
+ if (ctx->host)
+ pri_add_host_route(settings->interface, ctx->host);
}
static void append_context_properties(struct pri_context *ctx,
@@ -1366,7 +1367,7 @@ static void pri_context_destroy(gpointer userdata)
{
struct pri_context *ctx = userdata;
- g_free(ctx->proxy_host);
+ g_free(ctx->host);
g_free(ctx->path);
g_free(ctx);
}
@@ -3086,6 +3087,9 @@ unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
context->context.proto = proto;
+ if (host)
+ context->host = g_strdup(host);
+
gprs->last_context_id = id;
gprs->contexts = g_slist_append(gprs->contexts, context);
@@ -3121,6 +3125,9 @@ static void activate_request_callback(const struct ofono_error *error,
gc->settings->ipv4) {
pri_set_ipv4_addr(gc->settings->interface,
gc->settings->ipv4->ip);
+ if (ctx->host)
+ pri_add_host_route(gc->settings->interface,
+ ctx->host);
}
}
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 6/8] gprs: Add a host route for STK context type
2011-05-20 16:26 ` [PATCH v4 6/8] gprs: Add a host route for STK context type Philippe Nunes
@ 2011-06-01 6:26 ` Denis Kenzior
0 siblings, 0 replies; 17+ messages in thread
From: Denis Kenzior @ 2011-06-01 6:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4828 bytes --]
Hi Philippe,
On 05/20/2011 11:26 AM, Philippe Nunes wrote:
> ---
> src/gprs.c | 45 ++++++++++++++++++++++++++-------------------
> 1 files changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/src/gprs.c b/src/gprs.c
> index 86d95bc..a867ea1 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -141,8 +141,8 @@ struct pri_context {
> unsigned int id;
> char *path;
> char *key;
> - char *proxy_host;
> - uint16_t proxy_port;
> + char *host;
> + uint16_t port;
> DBusMessage *pending;
> struct ofono_gprs_primary_context context;
> struct ofono_gprs_context *context_driver;
> @@ -616,16 +616,16 @@ static void pri_parse_proxy(struct pri_context *ctx, const char *proxy)
> host += 3;
>
> if (strcasecmp(scheme, "https") == 0)
> - ctx->proxy_port = 443;
> + ctx->port = 443;
> else if (strcasecmp(scheme, "http") == 0)
> - ctx->proxy_port = 80;
> + ctx->port = 80;
> else {
> g_free(scheme);
> return;
> }
> } else {
> host = scheme;
> - ctx->proxy_port = 80;
> + ctx->port = 80;
> }
>
> path = strchr(host, '/');
> @@ -639,12 +639,12 @@ static void pri_parse_proxy(struct pri_context *ctx, const char *proxy)
>
> if (*end == '\0') {
> *port = '\0';
> - ctx->proxy_port = tmp;
> + ctx->port = tmp;
> }
> }
>
> - g_free(ctx->proxy_host);
> - ctx->proxy_host = g_strdup(host);
> + g_free(ctx->host);
> + ctx->host = g_strdup(host);
>
> g_free(scheme);
> }
> @@ -728,7 +728,7 @@ done:
> close(sk);
> }
>
I don't think you really need to mess with this at all. The host IP is
already stored nicely inside the stk pending_cmd structure. Converting
it to a string, g_strduping it and then making sure to g_free it seems
like a total waste of time. It would be better to simply make
pri_setproxy into a utility function and call it from stk.c directly.
> -static void pri_setproxy(const char *interface, const char *proxy)
> +static void pri_add_host_route(const char *interface, const char *host)
> {
> struct rtentry rt;
> struct sockaddr_in addr;
> @@ -747,7 +747,7 @@ static void pri_setproxy(const char *interface, const char *proxy)
>
> memset(&addr, 0, sizeof(addr));
> addr.sin_family = AF_INET;
> - addr.sin_addr.s_addr = inet_addr(proxy);
> + addr.sin_addr.s_addr = inet_addr(host);
> memcpy(&rt.rt_dst, &addr, sizeof(rt.rt_dst));
>
> memset(&addr, 0, sizeof(addr));
> @@ -761,7 +761,7 @@ static void pri_setproxy(const char *interface, const char *proxy)
> memcpy(&rt.rt_genmask, &addr, sizeof(rt.rt_genmask));
>
> if (ioctl(sk, SIOCADDRT, &rt) < 0)
> - ofono_error("Failed to add proxy host route");
> + ofono_error("Failed to add host route");
>
> close(sk);
> }
> @@ -788,12 +788,13 @@ static void pri_reset_context_settings(struct pri_context *ctx)
>
> pri_context_signal_settings(ctx, signal_ipv4, signal_ipv6);
>
> - if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_MMS) {
> + if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_MMS ||
> + ctx->type == OFONO_GPRS_CONTEXT_TYPE_STK) {
> pri_set_ipv4_addr(interface, NULL);
>
> - g_free(ctx->proxy_host);
> - ctx->proxy_host = NULL;
> - ctx->proxy_port = 0;
> + g_free(ctx->host);
> + ctx->host = NULL;
> + ctx->port = 0;
> }
>
> pri_ifupdown(interface, FALSE);
> @@ -811,12 +812,12 @@ static void pri_update_mms_context_settings(struct pri_context *ctx)
>
> pri_parse_proxy(ctx, ctx->message_proxy);
>
> - DBG("proxy %s port %u", ctx->proxy_host, ctx->proxy_port);
> + DBG("host %s port %u", ctx->host, ctx->port);
>
> pri_set_ipv4_addr(settings->interface, settings->ipv4->ip);
>
> - if (ctx->proxy_host)
> - pri_setproxy(settings->interface, ctx->proxy_host);
> + if (ctx->host)
> + pri_add_host_route(settings->interface, ctx->host);
> }
>
> static void append_context_properties(struct pri_context *ctx,
> @@ -1366,7 +1367,7 @@ static void pri_context_destroy(gpointer userdata)
> {
> struct pri_context *ctx = userdata;
>
> - g_free(ctx->proxy_host);
> + g_free(ctx->host);
> g_free(ctx->path);
> g_free(ctx);
> }
> @@ -3086,6 +3087,9 @@ unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
>
> context->context.proto = proto;
>
> + if (host)
> + context->host = g_strdup(host);
> +
> gprs->last_context_id = id;
> gprs->contexts = g_slist_append(gprs->contexts, context);
>
> @@ -3121,6 +3125,9 @@ static void activate_request_callback(const struct ofono_error *error,
> gc->settings->ipv4) {
> pri_set_ipv4_addr(gc->settings->interface,
> gc->settings->ipv4->ip);
> + if (ctx->host)
> + pri_add_host_route(gc->settings->interface,
> + ctx->host);
> }
> }
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 7/8] stk: Add support of the Setup event list proactive command
2011-05-20 16:26 [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Philippe Nunes
` (4 preceding siblings ...)
2011-05-20 16:26 ` [PATCH v4 6/8] gprs: Add a host route for STK context type Philippe Nunes
@ 2011-05-20 16:26 ` Philippe Nunes
2011-05-20 16:26 ` [PATCH v4 8/8] stk: Add read/write handlers Philippe Nunes
2011-06-01 5:26 ` [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Denis Kenzior
7 siblings, 0 replies; 17+ messages in thread
From: Philippe Nunes @ 2011-05-20 16:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5110 bytes --]
---
src/stk.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 98 insertions(+), 10 deletions(-)
diff --git a/src/stk.c b/src/stk.c
index d9f1e25..fbd93c3 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -85,6 +85,8 @@ struct ofono_stk {
struct stk_channel_data rx_buffer;
struct stk_channel_data tx_buffer;
gboolean link_on_demand;
+ gboolean report_data_available;
+ gboolean report_channel_status;
struct ofono_gprs *gprs;
struct stk_bearer_description bearer_desc;
};
@@ -489,6 +491,58 @@ static void emit_menu_changed(struct ofono_stk *stk)
g_dbus_send_message(conn, signal);
}
+static void event_download_envelope_cb(struct ofono_stk *stk, gboolean ok,
+ const unsigned char *data, int len)
+{
+ if (!ok) {
+ ofono_error("Event download to UICC failed");
+ return;
+ }
+
+ if (len)
+ ofono_error("Event download returned %i bytes of data",
+ len);
+
+ DBG("Event download to UICC reported no error");
+}
+
+static void stk_send_channel_status_event(struct ofono_stk *stk)
+{
+ struct stk_envelope e;
+
+ memset(&e, 0, sizeof(e));
+ e.type = STK_ENVELOPE_TYPE_EVENT_DOWNLOAD;
+ e.src = STK_DEVICE_IDENTITY_TYPE_TERMINAL;
+ e.event_download.type = STK_EVENT_TYPE_CHANNEL_STATUS;
+ e.event_download.channel_status.channel.id = stk->channel.id;
+ e.event_download.channel_status.channel.status = stk->channel.status;
+
+ if (stk->channel.status == STK_CHANNEL_PACKET_DATA_SERVICE_ACTIVATED)
+ memcpy(&e.event_download.channel_status.bearer_desc,
+ &stk->bearer_desc,
+ sizeof(struct stk_bearer_description));
+
+ if (stk_send_envelope(stk, &e, event_download_envelope_cb, 0))
+ event_download_envelope_cb(stk, FALSE, NULL, -1);
+}
+
+static void stk_send_data_available_event(struct ofono_stk *stk,
+ unsigned short data_available)
+{
+ struct stk_envelope e;
+
+ memset(&e, 0, sizeof(e));
+ e.type = STK_ENVELOPE_TYPE_EVENT_DOWNLOAD;
+ e.src = STK_DEVICE_IDENTITY_TYPE_TERMINAL;
+ e.event_download.type = STK_EVENT_TYPE_DATA_AVAILABLE;
+ e.event_download.data_available.channel.id = stk->channel.id;
+ e.event_download.data_available.channel.status = stk->channel.status;
+ e.event_download.data_available.channel_data_len = data_available;
+
+ if (stk_send_envelope(stk, &e, event_download_envelope_cb, 0))
+ event_download_envelope_cb(stk, FALSE, NULL, -1);
+}
+
static void ofono_stk_remove_pdp_context_cb(int error, void *data)
{
struct ofono_stk *stk = data;
@@ -503,12 +557,8 @@ static void ofono_stk_remove_pdp_context_cb(int error, void *data)
send_simple_response(stk, STK_RESULT_TYPE_NOT_CAPABLE);
else
send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
- } else {
- /*
- * TODO
- * Send Event channel status
- */
- }
+ } else if (stk->report_channel_status)
+ stk_send_channel_status_event(stk);
/* free the buffers even in case of error */
g_free(stk->rx_buffer.data.array);
@@ -607,10 +657,8 @@ static void ofono_stk_activate_pdp_context_cb(int error, const char *interface,
}
if (stk->pending_cmd == NULL) {
- /*
- * TODO
- * Send Event channel status
- */
+ if (stk->report_channel_status)
+ stk_send_channel_status_event(stk);
return;
}
@@ -2515,6 +2563,8 @@ static gboolean handle_command_refresh(const struct stk_command *cmd,
case 5:
case 6:
free_idle_mode_text(stk);
+ stk->report_data_available = FALSE;
+ stk->report_channel_status = FALSE;
__ofono_sim_refresh(sim, file_list, FALSE, TRUE);
break;
}
@@ -3140,6 +3190,39 @@ static gboolean handle_command_get_channel_status(const struct stk_command *cmd,
return TRUE;
}
+
+static gboolean handle_command_setup_event_list(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk)
+{
+ const struct stk_command_setup_event_list *sel = &cmd->setup_event_list;
+ unsigned int i;
+
+ stk->report_data_available = FALSE;
+ stk->report_channel_status = FALSE;
+
+ for (i = 0; i < sel->event_list.len; i++) {
+ switch (sel->event_list.list[i]) {
+ case STK_EVENT_TYPE_DATA_AVAILABLE:
+ DBG("Enable data available event");
+ stk->report_data_available = TRUE;
+ break;
+ case STK_EVENT_TYPE_CHANNEL_STATUS:
+ DBG("Enable channel status event");
+ stk->report_channel_status = TRUE;
+ break;
+ default:
+ DBG("Event type (%d) not supported",
+ sel->event_list.list[i]);
+ rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ return TRUE;
+ }
+ }
+
+ rsp->result.type = STK_RESULT_TYPE_SUCCESS;
+ return TRUE;
+}
+
static void stk_proactive_command_cancel(struct ofono_stk *stk)
{
if (stk->immediate_response)
@@ -3358,6 +3441,11 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
&rsp, stk);
break;
+ case STK_COMMAND_TYPE_SETUP_EVENT_LIST:
+ respond = handle_command_setup_event_list(stk->pending_cmd,
+ &rsp, stk);
+ break;
+
default:
rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
break;
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v4 8/8] stk: Add read/write handlers
2011-05-20 16:26 [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Philippe Nunes
` (5 preceding siblings ...)
2011-05-20 16:26 ` [PATCH v4 7/8] stk: Add support of the Setup event list proactive command Philippe Nunes
@ 2011-05-20 16:26 ` Philippe Nunes
2011-06-01 5:26 ` [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Denis Kenzior
7 siblings, 0 replies; 17+ messages in thread
From: Philippe Nunes @ 2011-05-20 16:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 7470 bytes --]
---
src/stk.c | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 161 insertions(+), 10 deletions(-)
diff --git a/src/stk.c b/src/stk.c
index fbd93c3..72ca936 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -29,6 +29,8 @@
#include <stdlib.h>
#include <stdint.h>
#include <arpa/inet.h>
+#include <unistd.h>
+#include <fcntl.h>
#include <glib.h>
#include <gdbus.h>
@@ -89,6 +91,10 @@ struct ofono_stk {
gboolean report_channel_status;
struct ofono_gprs *gprs;
struct stk_bearer_description bearer_desc;
+ enum stk_transport_protocol_type protocol;
+ struct sockaddr_in dest_addr;
+ GIOChannel *io;
+ guint read_watch;
};
struct envelope_op {
@@ -573,6 +579,9 @@ static void stk_close_channel(struct ofono_stk *stk)
{
int err;
+ if (stk->read_watch > 0)
+ g_source_remove(stk->read_watch);
+
err = __ofono_gprs_remove_pdp_context(stk->gprs, stk->channel.id,
ofono_stk_remove_pdp_context_cb, stk);
@@ -636,6 +645,74 @@ static void stk_alpha_id_unset(struct ofono_stk *stk)
stk_agent_request_cancel(stk->current_agent);
}
+gsize stk_channel_data_write(struct ofono_stk *stk)
+{
+ GIOStatus status;
+ gsize bytes_written;
+ gchar *data = (gchar *)(stk->tx_buffer.data.array);
+ gsize count = stk->tx_buffer.data.len - stk->tx_buffer.tx_avail;
+
+ if (stk->protocol == STK_TRANSPORT_PROTOCOL_UDP_CLIENT_REMOTE) {
+ bytes_written = sendto(g_io_channel_unix_get_fd(stk->io), data,
+ count, 0, &stk->dest_addr,
+ sizeof(stk->dest_addr));
+
+ if (bytes_written == -1 && stk->read_watch > 0) {
+ g_source_remove(stk->read_watch);
+ return 0;
+ }
+ } else {
+ status = g_io_channel_write_chars(stk->io, data,
+ count, &bytes_written, NULL);
+
+ if (status != G_IO_STATUS_NORMAL && stk->read_watch > 0) {
+ g_source_remove(stk->read_watch);
+ return 0;
+ }
+ }
+
+ DBG("Send %zd bytes", bytes_written);
+
+ stk->tx_buffer.tx_avail += bytes_written;
+ count = stk->tx_buffer.data.len - stk->rx_buffer.tx_avail;
+ if (count > 0)
+ memmove(stk->tx_buffer.data.array, stk->rx_buffer.data.array +
+ bytes_written, count);
+
+ return bytes_written;
+}
+
+static gboolean receive_callback(GIOChannel *channel, GIOCondition cond,
+ gpointer userdata)
+{
+ struct ofono_stk *stk = (struct ofono_stk *) userdata;
+ GIOStatus status;
+ gsize bytes_read;
+ gchar *buf = (gchar *)(stk->rx_buffer.data.array +
+ stk->rx_buffer.rx_remaining);
+ gsize count = stk->rx_buffer.data.len - stk->rx_buffer.rx_remaining;
+
+ if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP))
+ return FALSE;
+
+ if (cond & G_IO_IN) {
+ status = g_io_channel_read_chars(channel, buf, count,
+ &bytes_read, NULL);
+ DBG("Received %zd bytes", bytes_read);
+
+ if (bytes_read > 0 && stk->rx_buffer.rx_remaining == 0 &&
+ stk->report_data_available) {
+ stk_send_data_available_event(stk, bytes_read);
+ }
+
+ stk->rx_buffer.rx_remaining += bytes_read;
+
+ if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
+ return FALSE;
+ }
+ return TRUE;
+}
+
static void ofono_stk_activate_pdp_context_cb(int error, const char *interface,
const char *ip,
void *data)
@@ -643,6 +720,8 @@ static void ofono_stk_activate_pdp_context_cb(int error, const char *interface,
struct ofono_stk *stk = data;
struct stk_response rsp;
struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
+ GIOChannel *io;
+ int sk;
DBG("");
@@ -656,6 +735,67 @@ static void ofono_stk_activate_pdp_context_cb(int error, const char *interface,
stk->channel.status = STK_CHANNEL_PACKET_DATA_SERVICE_ACTIVATED;
}
+ if (stk->protocol == STK_TRANSPORT_PROTOCOL_TCP_CLIENT_REMOTE) {
+ sk = socket(AF_INET, SOCK_STREAM, 0);
+ if (sk < 0) {
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+
+ error = connect(sk, (struct sockaddr *) &stk->dest_addr,
+ sizeof(stk->dest_addr));
+
+ if (error < 0) {
+ close(sk);
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+ } else if (stk->protocol == STK_TRANSPORT_PROTOCOL_UDP_CLIENT_REMOTE) {
+ struct sockaddr_in addr;
+
+ sk = socket(AF_INET, SOCK_DGRAM, 0);
+ if (sk < 0) {
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+
+ memset(&addr, 0, sizeof(addr));
+ addr.sin_family = AF_INET;
+ addr.sin_addr.s_addr = htonl(INADDR_ANY);
+ addr.sin_port = htons(0);
+
+ error = bind(sk, (struct sockaddr *) &addr, sizeof(addr));
+ if (error < 0) {
+ close(sk);
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+ } else {
+ /* raw IP */
+ sk = open(interface, O_RDWR);
+ if (sk < 0)
+ return;
+ }
+
+ io = g_io_channel_unix_new(sk);
+ if (io == NULL) {
+ close(sk);
+ error = -ENOMEM;
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+
+ g_io_channel_set_close_on_unref(io, TRUE);
+ g_io_channel_set_flags(io, G_IO_FLAG_NONBLOCK, NULL);
+ g_io_channel_set_encoding(io, NULL, NULL);
+ g_io_channel_set_buffered(io, FALSE);
+
+ stk->read_watch = g_io_add_watch_full(io, G_PRIORITY_DEFAULT,
+ G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
+ receive_callback, stk, NULL);
+ stk->io = io;
+ g_io_channel_unref(io);
+
if (stk->pending_cmd == NULL) {
if (stk->report_channel_status)
stk_send_channel_status_event(stk);
@@ -670,11 +810,13 @@ static void ofono_stk_activate_pdp_context_cb(int error, const char *interface,
sizeof(struct stk_bearer_description));
} else if (stk->pending_cmd->type == STK_COMMAND_TYPE_SEND_DATA &&
stk->link_on_demand) {
- /*
- * TODO
- * send the data immediately, flush the tx buffer
- */
-
+ while (stk->tx_buffer.data.len - stk->tx_buffer.tx_avail) {
+ if (stk_channel_data_write(stk) == 0) {
+ ofono_error("Failed to send data");
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+ }
rsp.send_data.tx_avail = stk->tx_buffer.tx_avail;
}
@@ -711,6 +853,11 @@ static void stk_open_channel(struct ofono_stk *stk)
memset(host, 0, sizeof(host));
addr.s_addr = oc->data_dest_addr.addr.ipv4;
inet_ntop(AF_INET, &addr, host, INET_ADDRSTRLEN);
+
+ memset(&stk->dest_addr, 0, sizeof(stk->dest_addr));
+ stk->dest_addr.sin_family = AF_INET;
+ stk->dest_addr.sin_port = htons(oc->uti.port);
+ stk->dest_addr.sin_addr.s_addr = oc->data_dest_addr.addr.ipv4;
} else {
/*
* For now, only the bearer type "GPRS / UTRAN packet service /
@@ -760,6 +907,7 @@ static void stk_open_channel(struct ofono_stk *stk)
memcpy(&stk->bearer_desc, &oc->bearer_desc,
sizeof(struct stk_bearer_description));
+ stk->protocol = oc->uti.protocol;
stk->link_on_demand = (stk->pending_cmd->qualifier &
STK_OPEN_CHANNEL_FLAG_IMMEDIATE) ? FALSE : TRUE;
@@ -848,11 +996,14 @@ static void stk_send_data(struct ofono_stk *stk,
return;
} else {
- /*
- * TODO
- * send the data immediately, flush the tx buffer
- */
- rsp.send_data.tx_avail = stk->tx_buffer.data.len;
+ while (stk->tx_buffer.data.len - stk->tx_buffer.tx_avail) {
+ if (stk_channel_data_write(stk) == 0) {
+ ofono_error("Failed to send data");
+ rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+ goto out;
+ }
+ }
+ rsp.send_data.tx_avail = stk->tx_buffer.tx_avail;
}
out:
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response
2011-05-20 16:26 [PATCH v4 1/8] stk: Clear 'respond_on_exit' flag after sending the terminal response Philippe Nunes
` (6 preceding siblings ...)
2011-05-20 16:26 ` [PATCH v4 8/8] stk: Add read/write handlers Philippe Nunes
@ 2011-06-01 5:26 ` Denis Kenzior
7 siblings, 0 replies; 17+ messages in thread
From: Denis Kenzior @ 2011-06-01 5:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 232 bytes --]
Hi Philippe,
On 05/20/2011 11:26 AM, Philippe Nunes wrote:
> ---
> src/stk.c | 27 ++-------------------------
> 1 files changed, 2 insertions(+), 25 deletions(-)
>
Patch has been applied, thanks.
Regards,
-Denis
^ permalink raw reply [flat|nested] 17+ messages in thread