* [PATCH 1/4][RFC] stkutil: Refactor command parser error handling
@ 2010-06-16 15:26 Andrzej Zaborowski
2010-06-16 15:26 ` [PATCH 2/4] stk: Allow registering proactive command handlers Andrzej Zaborowski
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Andrzej Zaborowski @ 2010-06-16 15:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 28497 bytes --]
When parsing the full command fails but Command Details has been parsed,
return a struct stk_command containing this information and the type of
parsing problem found. We need the command details to be able to
even respond to the command.
This patch also makes the parser skip over unknown data objects found
in the BER-TLV, if they don't have Comprehension Required set.
The tests succeed but please check that it doesn't break the logic.
---
src/stkutil.c | 188 +++++++++++++++++++--------------------------------
src/stkutil.h | 8 ++
unit/test-stkutil.c | 38 ++++++++++-
3 files changed, 113 insertions(+), 121 deletions(-)
diff --git a/src/stkutil.c b/src/stkutil.c
index 4e11179..5a100b3 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -2121,16 +2121,17 @@ struct dataobj_handler_entry {
enum stk_data_object_type type;
int flags;
void *data;
- gboolean parsed;
};
-static gboolean parse_dataobj(struct comprehension_tlv_iter *iter,
- enum stk_data_object_type type, ...)
+static enum stk_command_parse_result parse_dataobj(
+ struct comprehension_tlv_iter *iter,
+ enum stk_data_object_type type, ...)
{
GSList *entries = NULL;
GSList *l;
va_list args;
gboolean minimum_set = TRUE;
+ gboolean parse_error = FALSE;
va_start(args, type);
@@ -2147,45 +2148,59 @@ static gboolean parse_dataobj(struct comprehension_tlv_iter *iter,
entries = g_slist_prepend(entries, entry);
}
- if (comprehension_tlv_iter_next(iter) != TRUE)
- goto out;
-
entries = g_slist_reverse(entries);
- for (l = entries; l; l = l->next) {
+ l = entries;
+ while (comprehension_tlv_iter_next(iter) == TRUE) {
dataobj_handler handler;
- struct dataobj_handler_entry *entry = l->data;
+ struct dataobj_handler_entry *entry;
+ GSList *l2;
+
+ for (l2 = l; l2; l2 = l2->next) {
+ entry = l2->data;
+
+ if (comprehension_tlv_iter_get_tag(iter) == entry->type)
+ break;
+
+ /* Can't skip over Minimum objects */
+ if (entry->flags & DATAOBJ_FLAG_MINIMUM)
+ l2 = NULL;
+ }
+
+ if (!l2) {
+ if (comprehension_tlv_get_cr(iter) == TRUE)
+ parse_error = TRUE;
+
+ continue;
+ }
if (entry->flags & DATAOBJ_FLAG_LIST)
handler = list_handler_for_type(entry->type);
else
handler = handler_for_type(entry->type);
- if (handler == NULL)
- continue;
-
- if (comprehension_tlv_iter_get_tag(iter) == entry->type) {
- if (handler(iter, entry->data))
- entry->parsed = TRUE;
+ if (handler(iter, entry->data) == FALSE)
+ parse_error = TRUE;
- if (comprehension_tlv_iter_next(iter) == FALSE)
- break;
- }
+ l = l2->next;
}
-out:
- for (l = entries; l; l = l->next) {
+ for (; l; l = l->next) {
struct dataobj_handler_entry *entry = l->data;
- if ((entry->flags & DATAOBJ_FLAG_MINIMUM) &&
- entry->parsed == FALSE)
+ if (entry->flags & DATAOBJ_FLAG_MINIMUM)
minimum_set = FALSE;
}
g_slist_foreach(entries, (GFunc)g_free, NULL);
g_slist_free(entries);
- return minimum_set;
+ if (minimum_set == FALSE)
+ return STK_PARSE_RESULT_MISSING_VALUE;
+ if (parse_error == TRUE)
+ return STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
+
+ return STK_PARSE_RESULT_OK;
}
static void destroy_display_text(struct stk_command *command)
@@ -2197,7 +2212,6 @@ static gboolean parse_display_text(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_display_text *obj = &command->display_text;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2205,7 +2219,7 @@ static gboolean parse_display_text(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_DISPLAY)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->text,
STK_DATA_OBJECT_TYPE_ICON_ID, 0,
@@ -2222,9 +2236,6 @@ static gboolean parse_display_text(struct stk_command *command,
command->destructor = destroy_display_text;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2237,7 +2248,6 @@ static gboolean parse_get_inkey(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_get_inkey *obj = &command->get_inkey;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2245,7 +2255,7 @@ static gboolean parse_get_inkey(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->text,
STK_DATA_OBJECT_TYPE_ICON_ID, 0,
@@ -2260,9 +2270,6 @@ static gboolean parse_get_inkey(struct stk_command *command,
command->destructor = destroy_get_inkey;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2276,7 +2283,6 @@ static gboolean parse_get_input(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_get_input *obj = &command->get_input;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2284,7 +2290,7 @@ static gboolean parse_get_input(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->text,
STK_DATA_OBJECT_TYPE_RESPONSE_LENGTH,
@@ -2302,9 +2308,6 @@ static gboolean parse_get_input(struct stk_command *command,
command->destructor = destroy_get_input;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2329,7 +2332,6 @@ static gboolean parse_play_tone(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_play_tone *obj = &command->play_tone;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2337,7 +2339,7 @@ static gboolean parse_play_tone(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_EARPIECE)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
&obj->alpha_id,
STK_DATA_OBJECT_TYPE_TONE, 0,
&obj->tone,
@@ -2353,9 +2355,6 @@ static gboolean parse_play_tone(struct stk_command *command,
command->destructor = destroy_play_tone;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2363,7 +2362,6 @@ static gboolean parse_poll_interval(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_poll_interval *obj = &command->poll_interval;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2371,14 +2369,11 @@ static gboolean parse_poll_interval(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_DURATION,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_DURATION,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->duration,
STK_DATA_OBJECT_TYPE_INVALID);
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2394,7 +2389,6 @@ static gboolean parse_setup_menu(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_setup_menu *obj = &command->setup_menu;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2404,7 +2398,7 @@ static gboolean parse_setup_menu(struct stk_command *command,
command->destructor = destroy_setup_menu;
- ret = parse_dataobj(iter,
+ command->status = parse_dataobj(iter,
STK_DATA_OBJECT_TYPE_ALPHA_ID,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->alpha_id,
@@ -2423,9 +2417,6 @@ static gboolean parse_setup_menu(struct stk_command *command,
&obj->item_text_attr_list,
STK_DATA_OBJECT_TYPE_INVALID);
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2441,7 +2432,6 @@ static gboolean parse_select_item(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_select_item *obj = &command->select_item;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2451,7 +2441,7 @@ static gboolean parse_select_item(struct stk_command *command,
command->destructor = destroy_select_item;
- ret = parse_dataobj(iter,
+ command->status = parse_dataobj(iter,
STK_DATA_OBJECT_TYPE_ALPHA_ID,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->alpha_id,
@@ -2474,9 +2464,6 @@ static gboolean parse_select_item(struct stk_command *command,
&obj->frame_id,
STK_DATA_OBJECT_TYPE_INVALID);
- if (ret == FALSE)
- return FALSE;
-
if (obj->items == NULL)
return FALSE;
@@ -2495,7 +2482,6 @@ static gboolean parse_send_sms(struct stk_command *command,
{
struct stk_command_send_sms *obj = &command->send_sms;
struct gsm_sms_tpdu gsm_tpdu;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2504,7 +2490,8 @@ static gboolean parse_send_sms(struct stk_command *command,
return FALSE;
memset(&gsm_tpdu, 0, sizeof(gsm_tpdu));
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+ command->status = parse_dataobj(iter,
+ STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
&obj->alpha_id,
STK_DATA_OBJECT_TYPE_ADDRESS, 0,
&obj->address,
@@ -2520,10 +2507,10 @@ static gboolean parse_send_sms(struct stk_command *command,
&obj->frame_id,
STK_DATA_OBJECT_TYPE_INVALID);
- command->destructor = destroy_send_sms;
+ if (command->status != STK_PARSE_RESULT_OK)
+ return TRUE;
- if (ret == FALSE)
- return FALSE;
+ command->destructor = destroy_send_sms;
if (gsm_tpdu.len == 0 && obj->cdma_sms.len == 0)
return FALSE;
@@ -2562,7 +2549,6 @@ static gboolean parse_setup_call(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_setup_call *obj = &command->setup_call;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2570,7 +2556,8 @@ static gboolean parse_setup_call(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+ command->status = parse_dataobj(iter,
+ STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
&obj->alpha_id_usr_cfm,
STK_DATA_OBJECT_TYPE_ADDRESS,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
@@ -2597,9 +2584,6 @@ static gboolean parse_setup_call(struct stk_command *command,
command->destructor = destroy_setup_call;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2614,7 +2598,6 @@ static gboolean parse_refresh(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_refresh *obj = &command->refresh;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2622,7 +2605,8 @@ static gboolean parse_refresh(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_FILE_LIST, 0,
+ command->status = parse_dataobj(iter,
+ STK_DATA_OBJECT_TYPE_FILE_LIST, 0,
&obj->file_list,
STK_DATA_OBJECT_TYPE_AID, 0,
&obj->aid,
@@ -2638,9 +2622,6 @@ static gboolean parse_refresh(struct stk_command *command,
command->destructor = destroy_refresh;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2672,7 +2653,6 @@ static gboolean parse_setup_event_list(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_setup_event_list *obj = &command->setup_event_list;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2680,14 +2660,11 @@ static gboolean parse_setup_event_list(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_EVENT_LIST,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_EVENT_LIST,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->event_list,
STK_DATA_OBJECT_TYPE_INVALID);
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2695,7 +2672,6 @@ static gboolean parse_perform_card_apdu(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_perform_card_apdu *obj = &command->perform_card_apdu;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2704,14 +2680,11 @@ static gboolean parse_perform_card_apdu(struct stk_command *command,
(command->dst > STK_DEVICE_IDENTITY_TYPE_CARD_READER_7))
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_C_APDU,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_C_APDU,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->c_apdu,
STK_DATA_OBJECT_TYPE_INVALID);
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2769,7 +2742,6 @@ static gboolean parse_timer_mgmt(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_timer_mgmt *obj = &command->timer_mgmt;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2777,16 +2749,13 @@ static gboolean parse_timer_mgmt(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TIMER_ID,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TIMER_ID,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->timer_id,
STK_DATA_OBJECT_TYPE_TIMER_VALUE, 0,
&obj->timer_value,
STK_DATA_OBJECT_TYPE_INVALID);
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2800,7 +2769,6 @@ static gboolean parse_setup_idle_mode_text(struct stk_command *command,
{
struct stk_command_setup_idle_mode_text *obj =
&command->setup_idle_mode_text;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2808,7 +2776,7 @@ static gboolean parse_setup_idle_mode_text(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_TEXT,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
&obj->text,
STK_DATA_OBJECT_TYPE_ICON_ID, 0,
@@ -2821,9 +2789,6 @@ static gboolean parse_setup_idle_mode_text(struct stk_command *command,
command->destructor = destroy_setup_idle_mode_text;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2837,7 +2802,6 @@ static gboolean parse_run_at_command(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_run_at_command *obj = &command->run_at_command;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2845,7 +2809,8 @@ static gboolean parse_run_at_command(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+ command->status = parse_dataobj(iter,
+ STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
&obj->alpha_id,
STK_DATA_OBJECT_TYPE_AT_COMMAND,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
@@ -2860,9 +2825,6 @@ static gboolean parse_run_at_command(struct stk_command *command,
command->destructor = destroy_run_at_command;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2876,7 +2838,6 @@ static gboolean parse_send_dtmf(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_send_dtmf *obj = &command->send_dtmf;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2884,7 +2845,8 @@ static gboolean parse_send_dtmf(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_NETWORK)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
+ command->status = parse_dataobj(iter,
+ STK_DATA_OBJECT_TYPE_ALPHA_ID, 0,
&obj->alpha_id,
STK_DATA_OBJECT_TYPE_DTMF_STRING,
DATAOBJ_FLAG_MANDATORY | DATAOBJ_FLAG_MINIMUM,
@@ -2899,9 +2861,6 @@ static gboolean parse_send_dtmf(struct stk_command *command,
command->destructor = destroy_send_dtmf;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2910,7 +2869,6 @@ static gboolean parse_language_notification(struct stk_command *command,
{
struct stk_command_language_notification *obj =
&command->language_notification;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2918,13 +2876,10 @@ static gboolean parse_language_notification(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_LANGUAGE, 0,
+ command->status = parse_dataobj(iter, STK_DATA_OBJECT_TYPE_LANGUAGE, 0,
&obj->language,
STK_DATA_OBJECT_TYPE_INVALID);
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -2946,7 +2901,6 @@ static gboolean parse_launch_browser(struct stk_command *command,
struct comprehension_tlv_iter *iter)
{
struct stk_command_launch_browser *obj = &command->launch_browser;
- gboolean ret;
if (command->src != STK_DEVICE_IDENTITY_TYPE_UICC)
return FALSE;
@@ -2954,7 +2908,7 @@ static gboolean parse_launch_browser(struct stk_command *command,
if (command->dst != STK_DEVICE_IDENTITY_TYPE_TERMINAL)
return FALSE;
- ret = parse_dataobj(iter,
+ command->status = parse_dataobj(iter,
STK_DATA_OBJECT_TYPE_BROWSER_ID, 0,
&obj->browser_id,
STK_DATA_OBJECT_TYPE_URL,
@@ -2985,9 +2939,6 @@ static gboolean parse_launch_browser(struct stk_command *command,
command->destructor = destroy_launch_browser;
- if (ret == FALSE)
- return FALSE;
-
return TRUE;
}
@@ -3122,17 +3073,16 @@ struct stk_command *stk_command_new_from_pdu(const unsigned char *pdu,
ok = parse_launch_browser(command, &iter);
break;
default:
- ok = FALSE;
- break;
+ command->status = STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD;
+ return command;
};
- if (ok)
- return command;
+ if (ok == FALSE)
+ command->status = STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD;
-fail:
- if (command->destructor)
- command->destructor(command);
+ return command;
+fail:
g_free(command);
return NULL;
diff --git a/src/stkutil.h b/src/stkutil.h
index f6a578a..0337f32 100644
--- a/src/stkutil.h
+++ b/src/stkutil.h
@@ -1166,12 +1166,20 @@ struct stk_command_launch_browser {
char *text_passwd;
};
+enum stk_command_parse_result {
+ STK_PARSE_RESULT_OK,
+ STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD,
+ STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD,
+ STK_PARSE_RESULT_MISSING_VALUE,
+};
+
struct stk_command {
unsigned char number;
unsigned char type;
unsigned char qualifier;
enum stk_device_identity_type src;
enum stk_device_identity_type dst;
+ enum stk_command_parse_result status;
union {
struct stk_command_display_text display_text;
diff --git a/unit/test-stkutil.c b/unit/test-stkutil.c
index a3d7bde..3c70413 100644
--- a/unit/test-stkutil.c
+++ b/unit/test-stkutil.c
@@ -716,6 +716,7 @@ static void test_display_text(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_DISPLAY_TEXT);
@@ -1653,6 +1654,7 @@ static void test_get_inkey(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_GET_INKEY);
@@ -2958,6 +2960,7 @@ static void test_get_input(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_GET_INPUT);
@@ -3003,6 +3006,7 @@ static void test_more_time(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_MORE_TIME);
@@ -4238,6 +4242,7 @@ static void test_play_tone(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_PLAY_TONE);
@@ -4286,6 +4291,7 @@ static void test_poll_interval(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_POLL_INTERVAL);
@@ -5507,6 +5513,7 @@ static void test_setup_menu(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_SETUP_MENU);
@@ -5531,6 +5538,17 @@ static void test_setup_menu(gconstpointer data)
stk_command_free(command);
}
+static void test_setup_menu_missing_val(gconstpointer data)
+{
+ const struct setup_menu_test *test = data;
+ struct stk_command *command;
+
+ command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
+
+ g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_MISSING_VALUE);
+}
+
static void test_setup_menu_neg(gconstpointer data)
{
const struct setup_menu_test *test = data;
@@ -5538,7 +5556,8 @@ static void test_setup_menu_neg(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
- g_assert(!command);
+ g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD);
}
struct select_item_test {
@@ -7091,6 +7110,7 @@ static void test_select_item(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_SELECT_ITEM);
@@ -8715,6 +8735,7 @@ static void test_send_sms(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_SEND_SMS);
@@ -9983,6 +10004,7 @@ static void test_setup_call(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_SETUP_CALL);
@@ -10057,6 +10079,7 @@ static void test_refresh(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_REFRESH);
@@ -10098,6 +10121,7 @@ static void test_polling_off(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_POLLING_OFF);
@@ -10183,6 +10207,7 @@ static void test_provide_local_info(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_PROVIDE_LOCAL_INFO);
@@ -10290,6 +10315,7 @@ static void test_setup_event_list(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_SETUP_EVENT_LIST);
@@ -10515,6 +10541,7 @@ static void test_perform_card_apdu(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_PERFORM_CARD_APDU);
@@ -10552,6 +10579,7 @@ static void test_get_reader_status(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_GET_READER_STATUS);
@@ -11072,6 +11100,7 @@ static void test_timer_mgmt(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_TIMER_MANAGEMENT);
@@ -11786,6 +11815,7 @@ static void test_setup_idle_mode_text(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_SETUP_IDLE_MODE_TEXT);
@@ -12540,6 +12570,7 @@ static void test_run_at_command(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_RUN_AT_COMMAND);
@@ -13205,6 +13236,7 @@ static void test_send_dtmf(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_SEND_DTMF);
@@ -13259,6 +13291,7 @@ static void test_language_notification(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_LANGUAGE_NOTIFICATION);
@@ -13917,6 +13950,7 @@ static void test_launch_browser(gconstpointer data)
command = stk_command_new_from_pdu(test->pdu, test->pdu_len);
g_assert(command);
+ g_assert(command->status == STK_PARSE_RESULT_OK);
g_assert(command->number == 1);
g_assert(command->type == STK_COMMAND_TYPE_LAUNCH_BROWSER);
@@ -20251,7 +20285,7 @@ int main(int argc, char **argv)
&setup_menu_data_913, test_setup_menu);
g_test_add_data_func("/teststk/Setup Menu Negative 1",
- &setup_menu_data_neg_1, test_setup_menu_neg);
+ &setup_menu_data_neg_1, test_setup_menu_missing_val);
g_test_add_data_func("/teststk/Setup Menu Negative 2",
&setup_menu_data_neg_2, test_setup_menu_neg);
g_test_add_data_func("/teststk/Setup Menu Negative 3",
--
1.7.1.86.g0e460.dirty
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] stk: Allow registering proactive command handlers.
2010-06-16 15:26 [PATCH 1/4][RFC] stkutil: Refactor command parser error handling Andrzej Zaborowski
@ 2010-06-16 15:26 ` Andrzej Zaborowski
2010-06-22 21:42 ` Denis Kenzior
2010-06-16 15:26 ` [PATCH 3/4] stk: Handle the More Time command as a nop Andrzej Zaborowski
` (2 subsequent siblings)
3 siblings, 1 reply; 7+ messages in thread
From: Andrzej Zaborowski @ 2010-06-16 15:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5513 bytes --]
---
src/stk.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 131 insertions(+), 13 deletions(-)
diff --git a/src/stk.c b/src/stk.c
index f472a63..1dd5b77 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -40,14 +40,85 @@
static GSList *g_drivers = NULL;
+typedef void (*generic_cb_t)(const struct ofono_error *error,
+ struct ofono_stk *stk);
+
+typedef void (*envelope_cb_t)(const struct ofono_error *error,
+ const unsigned char *rdata,
+ int length, struct ofono_stk *stk);
+
+typedef gboolean (*command_handler_t)(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk);
+
struct ofono_stk {
const struct ofono_stk_driver *driver;
void *driver_data;
struct ofono_atom *atom;
+ command_handler_t handlers[256];
+ struct stk_command *pending_cmd;
};
+static void stk_respond(struct ofono_stk *stk, struct stk_command *cmd,
+ struct stk_response *rsp, generic_cb_t cb)
+{
+ struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
+ const guint8 *tlv;
+ unsigned int tlv_len;
+
+ if (cmd == NULL)
+ cmd = stk->pending_cmd;
+ else
+ stk->pending_cmd = cmd;
+
+ rsp->src = STK_DEVICE_IDENTITY_TYPE_TERMINAL;
+ rsp->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
+ rsp->number = cmd->number;
+ rsp->type = cmd->type;
+ rsp->qualifier = cmd->qualifier;
+
+ if (stk->driver->terminal_response == NULL) {
+ cb(&error, stk);
+ return;
+ }
+
+ tlv = stk_pdu_from_response(rsp, &tlv_len);
+ if (!tlv) {
+ cb(&error, stk);
+ return;
+ }
+
+ stk->driver->terminal_response(stk, tlv_len, tlv,
+ (ofono_stk_generic_cb_t) cb, stk);
+}
+
+static void stk_send_envelope(struct ofono_stk *stk, struct stk_envelope *e,
+ envelope_cb_t cb)
+{
+ struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
+ const guint8 *tlv;
+ unsigned int tlv_len;
+
+ e->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
+
+ if (stk->driver->envelope == NULL) {
+ cb(&error, NULL, -1, stk);
+ return;
+ }
+
+ tlv = stk_pdu_from_envelope(e, &tlv_len);
+ if (!tlv) {
+ cb(&error, NULL, -1, stk);
+ return;
+ }
+
+ stk->driver->envelope(stk, tlv_len, tlv,
+ (ofono_stk_envelope_cb_t) cb, stk);
+}
+
static void stk_cbs_download_cb(const struct ofono_error *error,
- const unsigned char *data, int len, void *user)
+ const unsigned char *data, int len,
+ struct ofono_stk *stk)
{
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
ofono_error("CellBroadcast download to UICC failed");
@@ -56,34 +126,47 @@ static void stk_cbs_download_cb(const struct ofono_error *error,
return;
}
+ if (len)
+ ofono_error("CellBroadcast download returned %i bytes of data",
+ len);
+
DBG("CellBroadcast download to UICC reported no error");
}
void __ofono_cbs_sim_download(struct ofono_stk *stk, const struct cbs *msg)
{
- const guint8 *tlv;
- unsigned int tlv_len;
struct stk_envelope e;
- if (stk->driver->envelope == NULL)
- return;
+ memset(&e, 0, sizeof(e));
e.type = STK_ENVELOPE_TYPE_CBS_PP_DOWNLOAD;
e.src = STK_DEVICE_IDENTITY_TYPE_NETWORK;
- e.dst = STK_DEVICE_IDENTITY_TYPE_UICC;
memcpy(&e.cbs_pp_download.page, msg, sizeof(msg));
- tlv = stk_pdu_from_envelope(&e, &tlv_len);
- if (!tlv)
+ stk_send_envelope(stk, &e, stk_cbs_download_cb);
+}
+
+static void stk_command_cb(const struct ofono_error *error,
+ struct ofono_stk *stk)
+{
+ stk_command_free(stk->pending_cmd);
+ stk->pending_cmd = NULL;
+
+ if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+ ofono_error("TERMINAL RESPONSE to a UICC command failed");
+ /* "The ME may retry to deliver the same Cell Broadcast
+ * page." */
return;
+ }
- stk->driver->envelope(stk, tlv_len, tlv, stk_cbs_download_cb, stk);
+ DBG("TERMINAL RESPONSE to a command reported no errors");
}
void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
int length, const unsigned char *pdu)
{
struct stk_command *cmd;
+ struct stk_response rsp;
char *buf;
int i;
@@ -97,14 +180,49 @@ void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
ofono_error("Can't parse proactive command: %s", buf);
g_free(buf);
- /* TODO: return TERMINAL RESPONSE with permanent error */
+ /*
+ * There's nothing we can do, we'd need at least Command
+ * Details to respond with an error.
+ */
return;
}
+ g_free(buf);
- /* TODO: execute */
+ memset(&rsp, 0, sizeof(rsp));
- g_free(buf);
- stk_command_free(cmd);
+ switch (cmd->status) {
+ case STK_PARSE_RESULT_OK:
+ if (stk->handlers[cmd->type]) {
+ if (stk->handlers[cmd->type](cmd, &rsp, stk) == TRUE)
+ break;
+
+ stk->pending_cmd = cmd;
+ return;
+ }
+ /* Fall through */
+
+ case STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD:
+ default:
+ rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
+ break;
+
+ case STK_PARSE_RESULT_MISSING_VALUE:
+ rsp.result.type = STK_RESULT_TYPE_MINIMUM_NOT_MET;
+ break;
+
+ case STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD:
+ rsp.result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+ break;
+ }
+
+ stk_respond(stk, cmd, &rsp, stk_command_cb);
+}
+
+static void stk_command_handler_register(struct ofono_stk *stk,
+ enum stk_command_type type,
+ command_handler_t handler)
+{
+ stk->handlers[type] = handler;
}
int ofono_stk_driver_register(const struct ofono_stk_driver *d)
--
1.7.1.86.g0e460.dirty
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4] stk: Handle the More Time command as a nop.
2010-06-16 15:26 [PATCH 1/4][RFC] stkutil: Refactor command parser error handling Andrzej Zaborowski
2010-06-16 15:26 ` [PATCH 2/4] stk: Allow registering proactive command handlers Andrzej Zaborowski
@ 2010-06-16 15:26 ` Andrzej Zaborowski
2010-06-16 15:26 ` [PATCH 4/4] Add STK Menu D-bus interface Andrzej Zaborowski
2010-06-22 21:44 ` [PATCH 1/4][RFC] stkutil: Refactor command parser error handling Denis Kenzior
3 siblings, 0 replies; 7+ messages in thread
From: Andrzej Zaborowski @ 2010-06-16 15:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 878 bytes --]
---
src/stk.c | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/src/stk.c b/src/stk.c
index 1dd5b77..6138c8e 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -225,6 +225,15 @@ static void stk_command_handler_register(struct ofono_stk *stk,
stk->handlers[type] = handler;
}
+static gboolean handle_command_more_time(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk)
+{
+ /* Do nothing */
+
+ return TRUE;
+}
+
int ofono_stk_driver_register(const struct ofono_stk_driver *d)
{
DBG("driver: %p, name: %s", d, d->name);
@@ -295,6 +304,9 @@ struct ofono_stk *ofono_stk_create(struct ofono_modem *modem,
break;
}
+ stk_command_handler_register(stk, STK_COMMAND_TYPE_MORE_TIME,
+ handle_command_more_time);
+
return stk;
}
--
1.7.1.86.g0e460.dirty
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] Add STK Menu D-bus interface.
2010-06-16 15:26 [PATCH 1/4][RFC] stkutil: Refactor command parser error handling Andrzej Zaborowski
2010-06-16 15:26 ` [PATCH 2/4] stk: Allow registering proactive command handlers Andrzej Zaborowski
2010-06-16 15:26 ` [PATCH 3/4] stk: Handle the More Time command as a nop Andrzej Zaborowski
@ 2010-06-16 15:26 ` Andrzej Zaborowski
2010-06-22 21:44 ` [PATCH 1/4][RFC] stkutil: Refactor command parser error handling Denis Kenzior
3 siblings, 0 replies; 7+ messages in thread
From: Andrzej Zaborowski @ 2010-06-16 15:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 18830 bytes --]
This is tested on the F3607gw only, here's a proposed api (comments
welcome). The patch implements two STK commands: Menu Selection and
Set Up Menu. Set Up Menu just sets up a menu from which the user can
choose an option at any time, request help on an item etc, the menu
can change when a new Set Up Menu commands is issued by SIM. Menu
Selection does more or less the same thing, but the menu is only valid
for the period when the command is executing. The selected option or
"No response" result needs to be returned in TERMINAL RESPONSE, meaning
that the menu can't stay for too long because other STK communication
is blocked. (the F3607gw will even cancel the command on it's own
after some 20-30 seconds by sending us *STKEND - not handle yet)
ofono.org.SimApplication
Methods:
dict GetProperties()
Returns properties.
SetProperty(string name, int value)
Sets a property (only MenuTimeout is writable).
Select(int item_number)
Selects the given item, the number is a 0-based item
index in the MenuItems array. For Menu Selection this
terminates the command and brings back the default
menu (set up by Set Up Menu).
RequestHelp(int item_number)
Returns a help request to the card for the given menu
item. For Menu Selection this also terminates the
command.
GoBack()
Sends a Go Back response to the Menu Selection command.
(Only valid when the command is executing)
EndSession()
Sends a User Terminated Session response to the Menu
Selection command. (Only valid when the command is
executing)
Signals:
PropertyChanged(string name, variant value)
Properties:
string[] MenuItems
List of menu entries. When a Menu Selection command is
executing this is the temporary menu, after the command
terminates it goes back to presenting the global menu
given by Set Up Menu. An other possibility would be
for there to be a list of menu objects, each having a
list of items, title, help available indicator and
Select, GetHelp, GoBack, EndSession methods. This
would also avoid a race when a user calls Select() at
the same time the MenuItems changes.
string MenuTitle
bool HelpAvailable
int Timeout (read-write)
Only present when Menu Selection is executing. After
the given number of seconds from the command's start,
or from last write, expires, "No Response" is returned
to the card.
---
include/dbus.h | 1 +
src/stk.c | 520 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 521 insertions(+), 0 deletions(-)
diff --git a/include/dbus.h b/include/dbus.h
index d988760..6b03dd7 100644
--- a/include/dbus.h
+++ b/include/dbus.h
@@ -49,6 +49,7 @@ extern "C" {
#define OFONO_VOICECALL_MANAGER_INTERFACE "org.ofono.VoiceCallManager"
#define OFONO_DATA_CONNECTION_MANAGER_INTERFACE "org.ofono.DataConnectionManager"
#define OFONO_DATA_CONTEXT_INTERFACE "org.ofono.PrimaryDataContext"
+#define OFONO_SIM_APP_INTERFACE OFONO_SERVICE ".SimApplication"
/* Essentially a{sv} */
#define OFONO_PROPERTIES_ARRAY_SIGNATURE DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING \
diff --git a/src/stk.c b/src/stk.c
index 9a9d596..56bf573 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -51,12 +51,28 @@ typedef gboolean (*command_handler_t)(const struct stk_command *cmd,
struct stk_response *rsp,
struct ofono_stk *stk);
+struct stk_menu {
+ char *title;
+ GSList *items;
+ struct stk_items_next_action_indicator next_action;
+ int default_item;
+ struct stk_text_attribute title_attr;
+ struct stk_item_text_attribute_list item_attrs;
+ gboolean soft_key;
+ gboolean has_help;
+};
+
struct ofono_stk {
const struct ofono_stk_driver *driver;
void *driver_data;
struct ofono_atom *atom;
command_handler_t handlers[256];
struct stk_command *pending_cmd;
+ DBusMessage *pending;
+
+ struct stk_menu *default_menu;
+ struct stk_menu *current_menu; /* For Select Item menus */
+ int timeout;
};
static void stk_respond(struct ofono_stk *stk, struct stk_command *cmd,
@@ -226,6 +242,415 @@ static void stk_command_handler_register(struct ofono_stk *stk,
stk->handlers[type] = handler;
}
+static struct stk_menu *stk_menu_create(const char *title,
+ const struct stk_text_attribute *title_attr, GSList *items,
+ const struct stk_item_text_attribute_list *item_attrs,
+ const struct stk_items_next_action_indicator *next_action,
+ int default_item, gboolean soft_key, gboolean has_help)
+{
+ struct stk_menu *ret = g_new(struct stk_menu, 1);
+ GSList *l;
+
+ ret->title = g_strdup(title);
+ ret->default_item = default_item;
+ ret->items = g_slist_copy((GSList *) items);
+ memcpy(&ret->next_action, next_action, sizeof(ret->next_action));
+ memcpy(&ret->title_attr, title_attr, sizeof(ret->title_attr));
+ memcpy(&ret->item_attrs, item_attrs, sizeof(ret->item_attrs));
+ ret->soft_key = soft_key;
+ ret->has_help = has_help;
+
+ for (l = ret->items; l; l = l->next) {
+ struct stk_item *i = g_memdup(l->data, sizeof(*i));
+
+ i->text = g_strdup(i->text);
+ l->data = i;
+ }
+
+ return ret;
+}
+
+static void stk_menu_free(struct stk_menu *menu)
+{
+ GSList *l;
+
+ for (l = menu->items; l; l = l->next) {
+ struct stk_item *i = l->data;
+
+ g_free(i->text);
+ g_free(i);
+ }
+
+ g_slist_free(menu->items);
+ g_free(menu->title);
+ g_free(menu);
+}
+
+static char **stk_menu_items_strlist(struct stk_menu *menu)
+{
+ char **items = g_new0(char *, g_slist_length(menu->items) + 1);
+ int i;
+ GSList *l;
+
+ for (i = 0, l = menu->items; l; l = l->next, i++) {
+ struct stk_item *item = l->data;
+ items[i] = item->text;
+ }
+
+ return items;
+}
+
+static void emit_menu_changed(struct ofono_stk *stk)
+{
+ static struct stk_menu no_menu = {
+ .title = "",
+ .items = NULL,
+ .has_help = FALSE,
+ .default_item = -1,
+ };
+ DBusConnection *conn = ofono_dbus_get_connection();
+ const char *path = __ofono_atom_get_path(stk->atom);
+ char **items;
+ struct stk_menu *menu = &no_menu;
+ dbus_uint16_t timeout = stk->timeout;
+ dbus_bool_t boolval;
+ uint8_t byteval;
+
+
+ if (stk->default_menu)
+ menu = stk->default_menu;
+ if (stk->current_menu)
+ menu = stk->current_menu;
+
+ if (stk->current_menu)
+ ofono_dbus_signal_property_changed(conn, path,
+ OFONO_SIM_APP_INTERFACE,
+ "MenuTimeout",
+ DBUS_TYPE_UINT16, &timeout);
+
+ items = stk_menu_items_strlist(menu);
+ ofono_dbus_signal_array_property_changed(conn, path,
+ OFONO_SIM_APP_INTERFACE,
+ "MenuItems",
+ DBUS_TYPE_STRING, &items);
+ g_free(items);
+
+ ofono_dbus_signal_property_changed(conn, path,
+ OFONO_SIM_APP_INTERFACE,
+ "MenuTitle",
+ DBUS_TYPE_STRING, &menu->title);
+
+ boolval = menu->has_help;
+ ofono_dbus_signal_property_changed(conn, path,
+ OFONO_SIM_APP_INTERFACE,
+ "HelpAvailable",
+ DBUS_TYPE_BOOLEAN, &boolval);
+
+ if (menu->default_item >= 0) {
+ byteval = menu->default_item;
+ ofono_dbus_signal_property_changed(conn, path,
+ OFONO_SIM_APP_INTERFACE,
+ "DefaultSelection",
+ DBUS_TYPE_BYTE, &byteval);
+ }
+}
+
+static DBusMessage *stk_get_properties(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ struct ofono_stk *stk = data;
+ DBusMessage *reply;
+ DBusMessageIter iter;
+ DBusMessageIter dict;
+ char **items;
+ struct stk_menu *menu = stk->default_menu;
+ dbus_uint16_t timeout = stk->timeout;
+ dbus_bool_t boolval;
+ uint8_t byteval;
+
+ if (stk->current_menu)
+ menu = stk->current_menu;
+
+ reply = dbus_message_new_method_return(msg);
+ if (!reply)
+ return NULL;
+
+ dbus_message_iter_init_append(reply, &iter);
+
+ dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+ OFONO_PROPERTIES_ARRAY_SIGNATURE,
+ &dict);
+
+ if (!menu)
+ goto done;
+
+ if (stk->current_menu)
+ ofono_dbus_dict_append(&dict, "MenuTimeout",
+ DBUS_TYPE_UINT16, &timeout);
+
+ items = stk_menu_items_strlist(menu);
+ ofono_dbus_dict_append_array(&dict, "MenuItems",
+ DBUS_TYPE_STRING, &items);
+ g_free(items);
+
+ ofono_dbus_dict_append(&dict, "MenuTitle",
+ DBUS_TYPE_STRING, &menu->title);
+
+ boolval = menu->has_help;
+ ofono_dbus_dict_append(&dict, "HelpAvailable",
+ DBUS_TYPE_BOOLEAN, &boolval);
+
+ if (menu->default_item >= 0) {
+ byteval = menu->default_item;
+ ofono_dbus_dict_append(&dict, "DefaultSelection",
+ DBUS_TYPE_BYTE, &byteval);
+ }
+
+done:
+ dbus_message_iter_close_container(&iter, &dict);
+
+ return reply;
+}
+
+static DBusMessage *stk_set_property(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ struct ofono_stk *stk = data;
+ const char *path = __ofono_atom_get_path(stk->atom);
+ DBusMessageIter iter;
+ DBusMessageIter var;
+ const char *property;
+ dbus_uint16_t timeout;
+
+ if (!dbus_message_iter_init(msg, &iter))
+ return __ofono_error_invalid_args(msg);
+
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+ return __ofono_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&iter, &property);
+ if (!dbus_message_iter_next(&iter))
+ return __ofono_error_invalid_args(msg);
+
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+ return __ofono_error_invalid_args(msg);
+
+ dbus_message_iter_recurse(&iter, &var);
+
+ if (stk->current_menu && !strcmp(property, "MenuTimeout")) {
+ if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_UINT16)
+ return __ofono_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&var, &timeout);
+ if (dbus_message_iter_next(&iter))
+ return __ofono_error_invalid_args(msg);
+
+ stk->timeout = timeout;
+ /* TODO: reschedule */
+
+ ofono_dbus_signal_property_changed(conn, path,
+ OFONO_SIM_APP_INTERFACE,
+ "MenuTimeout",
+ DBUS_TYPE_UINT16, &timeout);
+
+ return dbus_message_new_method_return(msg);
+ }
+
+ return __ofono_error_invalid_args(msg);
+}
+
+static void select_item_cb(const struct ofono_error *error,
+ struct ofono_stk *stk)
+{
+ if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+ DBG("Selecting item returned error: %s",
+ telephony_error_to_str(error));
+
+ __ofono_dbus_pending_reply(&stk->pending,
+ __ofono_error_failed(stk->pending));
+ return;
+ }
+
+ __ofono_dbus_pending_reply(&stk->pending,
+ dbus_message_new_method_return(stk->pending));
+
+ stk_command_free(stk->pending_cmd);
+ stk->pending_cmd = NULL;
+
+ stk_menu_free(stk->current_menu);
+ stk->current_menu = NULL;
+
+ emit_menu_changed(stk);
+}
+
+static void select_item_respond(struct ofono_stk *stk, struct stk_item *item,
+ gboolean help)
+{
+ struct stk_response rsp;
+
+ memset(&rsp, 0, sizeof(rsp));
+
+ if (help)
+ rsp.result.type = STK_RESULT_TYPE_HELP_REQUESTED;
+
+ rsp.select_item.item_id = item->id;
+
+ stk_respond(stk, NULL, &rsp, select_item_cb);
+}
+
+static void menu_selection_cb(const struct ofono_error *error,
+ const unsigned char *data, int len,
+ struct ofono_stk *stk)
+{
+ if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+ DBG("Selecting item returned error: %s",
+ telephony_error_to_str(error));
+
+ __ofono_dbus_pending_reply(&stk->pending,
+ __ofono_error_failed(stk->pending));
+ return;
+ }
+
+ if (len)
+ ofono_error("Selecting item returned %i bytes of data", len);
+
+ __ofono_dbus_pending_reply(&stk->pending,
+ dbus_message_new_method_return(stk->pending));
+}
+
+static void send_menu_selection(struct ofono_stk *stk, struct stk_item *item,
+ gboolean help)
+{
+ struct stk_envelope e;
+
+ memset(&e, 0, sizeof(e));
+
+ e.type = STK_ENVELOPE_TYPE_MENU_SELECTION;
+ e.src = STK_DEVICE_IDENTITY_TYPE_KEYPAD,
+ e.menu_selection.item_id = item->id;
+ e.menu_selection.help_request = help;
+
+ stk_send_envelope(stk, &e, menu_selection_cb);
+}
+
+static DBusMessage *stk_menu_select(DBusConnection *conn, DBusMessage *msg,
+ void *data, gboolean help)
+{
+ struct ofono_stk *stk = data;
+ DBusMessageIter iter;
+ uint8_t selection;
+ struct stk_menu *menu;
+ struct stk_item *item;
+
+ if (stk->pending)
+ return __ofono_error_busy(msg);
+
+ if (!dbus_message_iter_init(msg, &iter))
+ return __ofono_error_invalid_args(msg);
+
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_BYTE)
+ return __ofono_error_invalid_args(msg);
+
+ dbus_message_iter_get_basic(&iter, &selection);
+ if (dbus_message_iter_next(&iter))
+ return __ofono_error_invalid_args(msg);
+
+ menu = stk->default_menu;
+ if (stk->current_menu)
+ menu = stk->current_menu;
+
+ if (!menu)
+ return __ofono_error_failed(msg);
+
+ if (help == TRUE && !menu->has_help)
+ return __ofono_error_failed(msg);
+
+ item = g_slist_nth_data(menu->items, selection);
+ if (!item)
+ return __ofono_error_invalid_args(msg);
+
+ stk->pending = dbus_message_ref(msg);
+
+ if (stk->current_menu)
+ select_item_respond(stk, item, help);
+ else
+ send_menu_selection(stk, item, help);
+
+ return NULL;
+}
+
+static DBusMessage *stk_select(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ return stk_menu_select(conn, msg, data, FALSE);
+}
+
+static DBusMessage *stk_get_help(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ return stk_menu_select(conn, msg, data, TRUE);
+}
+
+static DBusMessage *stk_back_or_terminate(DBusConnection *conn,
+ DBusMessage *msg, void *data,
+ enum stk_result_type type)
+{
+ struct ofono_stk *stk = data;
+ DBusMessageIter iter;
+ struct stk_response rsp;
+
+ if (stk->pending)
+ return __ofono_error_busy(msg);
+
+ if (!stk->pending_cmd)
+ return __ofono_error_failed(msg);
+
+ if (dbus_message_iter_init(msg, &iter))
+ return __ofono_error_invalid_args(msg);
+
+ stk->pending = dbus_message_ref(msg);
+
+ memset(&rsp, 0, sizeof(rsp));
+ rsp.result.type = type;
+
+ stk_respond(stk, NULL, &rsp, select_item_cb);
+
+ return NULL;
+}
+
+static DBusMessage *stk_go_back(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ return stk_back_or_terminate(conn, msg, data,
+ STK_RESULT_TYPE_GO_BACK);
+}
+
+static DBusMessage *stk_end_session(DBusConnection *conn,
+ DBusMessage *msg, void *data)
+{
+ return stk_back_or_terminate(conn, msg, data,
+ STK_RESULT_TYPE_USER_TERMINATED);
+}
+
+static GDBusMethodTable stk_methods[] = {
+ { "GetProperties", "", "a{sv}", stk_get_properties },
+ { "SetProperty", "sv", "", stk_set_property },
+ { "Select", "y", "", stk_select,
+ G_DBUS_METHOD_FLAG_ASYNC },
+ { "RequestHelp", "y", "", stk_get_help,
+ G_DBUS_METHOD_FLAG_ASYNC },
+ { "GoBack", "", "", stk_go_back,
+ G_DBUS_METHOD_FLAG_ASYNC },
+ { "EndSession", "", "", stk_end_session,
+ G_DBUS_METHOD_FLAG_ASYNC },
+ { }
+};
+
+static GDBusSignalTable stk_signals[] = {
+ { "PropertyChanged", "sv" },
+ { }
+};
+
static gboolean handle_command_more_time(const struct stk_command *cmd,
struct stk_response *rsp,
struct ofono_stk *stk)
@@ -235,6 +660,57 @@ static gboolean handle_command_more_time(const struct stk_command *cmd,
return TRUE;
}
+static gboolean handle_command_select_item(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk)
+{
+ gboolean soft_key = (cmd->qualifier & (1 << 2)) != 0;
+ gboolean has_help = (cmd->qualifier & (1 << 7)) != 0;
+
+ stk->current_menu = stk_menu_create(cmd->select_item.alpha_id,
+ &cmd->select_item.text_attr,
+ cmd->select_item.items,
+ &cmd->select_item.item_text_attr_list,
+ &cmd->select_item.next_act,
+ cmd->select_item.item_id,
+ soft_key, has_help);
+
+ emit_menu_changed(stk);
+
+ /* TODO: schedule timeout */
+
+ return FALSE;
+}
+
+static gboolean handle_command_set_up_menu(const struct stk_command *cmd,
+ struct stk_response *rsp,
+ struct ofono_stk *stk)
+{
+ gboolean soft_key = (cmd->qualifier & (1 << 0)) != 0;
+ gboolean has_help = (cmd->qualifier & (1 << 7)) != 0;
+
+ if (stk->default_menu) {
+ stk_menu_free(stk->default_menu);
+ stk->default_menu = NULL;
+ }
+
+ if (cmd->setup_menu.items == NULL)
+ goto out;
+
+ stk->default_menu = stk_menu_create(cmd->setup_menu.alpha_id,
+ &cmd->setup_menu.text_attr,
+ cmd->setup_menu.items,
+ &cmd->setup_menu.item_text_attr_list,
+ &cmd->setup_menu.next_act,
+ -1, soft_key, has_help);
+
+out:
+ if (stk->current_menu == NULL)
+ emit_menu_changed(stk);
+
+ return TRUE;
+}
+
int ofono_stk_driver_register(const struct ofono_stk_driver *d)
{
DBG("driver: %p, name: %s", d, d->name);
@@ -256,6 +732,29 @@ void ofono_stk_driver_unregister(const struct ofono_stk_driver *d)
static void stk_unregister(struct ofono_atom *atom)
{
+ struct ofono_stk *stk = __ofono_atom_get_data(atom);
+ DBusConnection *conn = ofono_dbus_get_connection();
+ struct ofono_modem *modem = __ofono_atom_get_modem(atom);
+ const char *path = __ofono_atom_get_path(atom);
+
+ if (stk->pending_cmd) {
+ stk_command_free(stk->pending_cmd);
+ stk->pending_cmd = NULL;
+ /* TODO: potentially cancel the timeout */
+ }
+
+ if (stk->current_menu) {
+ stk_menu_free(stk->current_menu);
+ stk->current_menu = NULL;
+ }
+
+ if (stk->default_menu) {
+ stk_menu_free(stk->default_menu);
+ stk->default_menu = NULL;
+ }
+
+ ofono_modem_remove_interface(modem, OFONO_SIM_APP_INTERFACE);
+ g_dbus_unregister_interface(conn, path, OFONO_SIM_APP_INTERFACE);
}
static void stk_remove(struct ofono_atom *atom)
@@ -305,14 +804,35 @@ struct ofono_stk *ofono_stk_create(struct ofono_modem *modem,
break;
}
+ stk->timeout = 20;
+
stk_command_handler_register(stk, STK_COMMAND_TYPE_MORE_TIME,
handle_command_more_time);
+ stk_command_handler_register(stk, STK_COMMAND_TYPE_SELECT_ITEM,
+ handle_command_select_item);
+ stk_command_handler_register(stk, STK_COMMAND_TYPE_SETUP_MENU,
+ handle_command_set_up_menu);
return stk;
}
void ofono_stk_register(struct ofono_stk *stk)
{
+ DBusConnection *conn = ofono_dbus_get_connection();
+ struct ofono_modem *modem = __ofono_atom_get_modem(stk->atom);
+ const char *path = __ofono_atom_get_path(stk->atom);
+
+ if (!g_dbus_register_interface(conn, path, OFONO_SIM_APP_INTERFACE,
+ stk_methods, stk_signals, NULL,
+ stk, NULL)) {
+ ofono_error("Could not create %s interface",
+ OFONO_SIM_APP_INTERFACE);
+
+ return;
+ }
+
+ ofono_modem_add_interface(modem, OFONO_SIM_APP_INTERFACE);
+
__ofono_atom_register(stk->atom, stk_unregister);
}
--
1.7.1.86.g0e460.dirty
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] stk: Allow registering proactive command handlers.
2010-06-16 15:26 ` [PATCH 2/4] stk: Allow registering proactive command handlers Andrzej Zaborowski
@ 2010-06-22 21:42 ` Denis Kenzior
2010-06-22 22:14 ` Andrzej Zaborowski
0 siblings, 1 reply; 7+ messages in thread
From: Denis Kenzior @ 2010-06-22 21:42 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 6737 bytes --]
Hi Andrew,
> ---
> src/stk.c | 144
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1
files
> changed, 131 insertions(+), 13 deletions(-)
>
> diff --git a/src/stk.c b/src/stk.c
> index f472a63..1dd5b77 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -40,14 +40,85 @@
>
> static GSList *g_drivers = NULL;
>
> +typedef void (*generic_cb_t)(const struct ofono_error *error,
> + struct ofono_stk *stk);
> +
> +typedef void (*envelope_cb_t)(const struct ofono_error *error,
> + const unsigned char *rdata,
> + int length, struct ofono_stk *stk);
User data?
> +
> +typedef gboolean (*command_handler_t)(const struct stk_command *cmd,
> + struct stk_response *rsp,
> + struct ofono_stk *stk);
I think here we must have user data, especially if you're planning to use it
outside stk (e.g. in voicecall atom). Any reason why *stk can't simply be
void *?
> +
> struct ofono_stk {
> const struct ofono_stk_driver *driver;
> void *driver_data;
> struct ofono_atom *atom;
> + command_handler_t handlers[256];
There are only 41 proactive commands, and we'll implement a subset. This
seems a bit excessive. However, if we can't come up with anything more
clever, I'm actually fine with it for fast access.
> + struct stk_command *pending_cmd;
> };
>
> +static void stk_respond(struct ofono_stk *stk, struct stk_command *cmd,
> + struct stk_response *rsp, generic_cb_t cb)
> +{
Can we avoid passing in the original stk_command object? Since only one
command is pending at a time, sounds like we should be able to set it
elsewhere easily.
Is the plan to make this public API at some point? If so it definitely
requires userdata.
> + struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
> + const guint8 *tlv;
> + unsigned int tlv_len;
> +
> + if (cmd == NULL)
> + cmd = stk->pending_cmd;
> + else
> + stk->pending_cmd = cmd;
> +
> + rsp->src = STK_DEVICE_IDENTITY_TYPE_TERMINAL;
> + rsp->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
> + rsp->number = cmd->number;
> + rsp->type = cmd->type;
> + rsp->qualifier = cmd->qualifier;
> +
> + if (stk->driver->terminal_response == NULL) {
> + cb(&error, stk);
> + return;
> + }
> +
> + tlv = stk_pdu_from_response(rsp, &tlv_len);
> + if (!tlv) {
> + cb(&error, stk);
> + return;
> + }
> +
> + stk->driver->terminal_response(stk, tlv_len, tlv,
> + (ofono_stk_generic_cb_t) cb, stk);
> +}
> +
> +static void stk_send_envelope(struct ofono_stk *stk, struct stk_envelope
> *e, + envelope_cb_t cb)
Same comments here. Should this be public API with userdata?
> +{
> + struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
> + const guint8 *tlv;
> + unsigned int tlv_len;
> +
> + e->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
> +
> + if (stk->driver->envelope == NULL) {
> + cb(&error, NULL, -1, stk);
> + return;
> + }
> +
> + tlv = stk_pdu_from_envelope(e, &tlv_len);
> + if (!tlv) {
> + cb(&error, NULL, -1, stk);
> + return;
> + }
> +
> + stk->driver->envelope(stk, tlv_len, tlv,
> + (ofono_stk_envelope_cb_t) cb, stk);
> +}
> +
> static void stk_cbs_download_cb(const struct ofono_error *error,
> - const unsigned char *data, int len, void *user)
> + const unsigned char *data, int len,
> + struct ofono_stk *stk)
> {
> if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> ofono_error("CellBroadcast download to UICC failed");
> @@ -56,34 +126,47 @@ static void stk_cbs_download_cb(const struct
> ofono_error *error, return;
> }
>
> + if (len)
> + ofono_error("CellBroadcast download returned %i bytes of data",
> + len);
> +
> DBG("CellBroadcast download to UICC reported no error");
> }
>
> void __ofono_cbs_sim_download(struct ofono_stk *stk, const struct cbs
> *msg) {
> - const guint8 *tlv;
> - unsigned int tlv_len;
> struct stk_envelope e;
>
> - if (stk->driver->envelope == NULL)
> - return;
> + memset(&e, 0, sizeof(e));
>
> e.type = STK_ENVELOPE_TYPE_CBS_PP_DOWNLOAD;
> e.src = STK_DEVICE_IDENTITY_TYPE_NETWORK;
> - e.dst = STK_DEVICE_IDENTITY_TYPE_UICC;
> memcpy(&e.cbs_pp_download.page, msg, sizeof(msg));
>
> - tlv = stk_pdu_from_envelope(&e, &tlv_len);
> - if (!tlv)
> + stk_send_envelope(stk, &e, stk_cbs_download_cb);
> +}
> +
> +static void stk_command_cb(const struct ofono_error *error,
> + struct ofono_stk *stk)
> +{
> + stk_command_free(stk->pending_cmd);
> + stk->pending_cmd = NULL;
> +
> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> + ofono_error("TERMINAL RESPONSE to a UICC command failed");
> + /* "The ME may retry to deliver the same Cell Broadcast
> + * page." */
> return;
> + }
>
> - stk->driver->envelope(stk, tlv_len, tlv, stk_cbs_download_cb, stk);
> + DBG("TERMINAL RESPONSE to a command reported no errors");
> }
>
> void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
> int length, const unsigned char *pdu)
> {
> struct stk_command *cmd;
> + struct stk_response rsp;
> char *buf;
> int i;
>
> @@ -97,14 +180,49 @@ void ofono_stk_proactive_command_notify(struct
> ofono_stk *stk, ofono_error("Can't parse proactive command: %s", buf);
> g_free(buf);
>
> - /* TODO: return TERMINAL RESPONSE with permanent error */
> + /*
> + * There's nothing we can do, we'd need at least Command
> + * Details to respond with an error.
> + */
> return;
> }
> + g_free(buf);
>
> - /* TODO: execute */
> + memset(&rsp, 0, sizeof(rsp));
>
> - g_free(buf);
> - stk_command_free(cmd);
> + switch (cmd->status) {
> + case STK_PARSE_RESULT_OK:
> + if (stk->handlers[cmd->type]) {
> + if (stk->handlers[cmd->type](cmd, &rsp, stk) == TRUE)
> + break;
> +
> + stk->pending_cmd = cmd;
> + return;
> + }
> + /* Fall through */
> +
> + case STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD:
> + default:
> + rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
> + break;
> +
> + case STK_PARSE_RESULT_MISSING_VALUE:
> + rsp.result.type = STK_RESULT_TYPE_MINIMUM_NOT_MET;
> + break;
> +
> + case STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD:
> + rsp.result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
> + break;
> + }
> +
> + stk_respond(stk, cmd, &rsp, stk_command_cb);
> +}
> +
> +static void stk_command_handler_register(struct ofono_stk *stk,
> + enum stk_command_type type,
> + command_handler_t handler)
> +{
> + stk->handlers[type] = handler;
More error handling, might want to return an error if something is already
registered.
> }
>
> int ofono_stk_driver_register(const struct ofono_stk_driver *d)
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4][RFC] stkutil: Refactor command parser error handling
2010-06-16 15:26 [PATCH 1/4][RFC] stkutil: Refactor command parser error handling Andrzej Zaborowski
` (2 preceding siblings ...)
2010-06-16 15:26 ` [PATCH 4/4] Add STK Menu D-bus interface Andrzej Zaborowski
@ 2010-06-22 21:44 ` Denis Kenzior
3 siblings, 0 replies; 7+ messages in thread
From: Denis Kenzior @ 2010-06-22 21:44 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 608 bytes --]
Hi Andrew,
> When parsing the full command fails but Command Details has been parsed,
> return a struct stk_command containing this information and the type of
> parsing problem found. We need the command details to be able to
> even respond to the command.
>
> This patch also makes the parser skip over unknown data objects found
> in the BER-TLV, if they don't have Comprehension Required set.
>
> The tests succeed but please check that it doesn't break the logic.
I'm ok with the approach, however the patch no longer applies. Can you please
rebase and resubmit?
Thanks,
-Denis
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] stk: Allow registering proactive command handlers.
2010-06-22 21:42 ` Denis Kenzior
@ 2010-06-22 22:14 ` Andrzej Zaborowski
0 siblings, 0 replies; 7+ messages in thread
From: Andrzej Zaborowski @ 2010-06-22 22:14 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 9589 bytes --]
On 22 June 2010 23:42, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Andrew,
>
>> ---
>> src/stk.c | 144
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1
> files
>> changed, 131 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/stk.c b/src/stk.c
>> index f472a63..1dd5b77 100644
>> --- a/src/stk.c
>> +++ b/src/stk.c
>> @@ -40,14 +40,85 @@
>>
>> static GSList *g_drivers = NULL;
>>
>> +typedef void (*generic_cb_t)(const struct ofono_error *error,
>> + struct ofono_stk *stk);
>> +
>> +typedef void (*envelope_cb_t)(const struct ofono_error *error,
>> + const unsigned char *rdata,
>> + int length, struct ofono_stk *stk);
>
> User data?
Changed to just "data", as this is "response data".
>
>> +
>> +typedef gboolean (*command_handler_t)(const struct stk_command *cmd,
>> + struct stk_response *rsp,
>> + struct ofono_stk *stk);
>
> I think here we must have user data, especially if you're planning to use it
> outside stk (e.g. in voicecall atom). Any reason why *stk can't simply be
> void *?
Well, these types are just copies of the types in include/ofono/stk.h,
which do take void *, so these ones are just for local use in this
file to make working with the stk_respond / stk_send_envelope
functions easier.
>
>> +
>> struct ofono_stk {
>> const struct ofono_stk_driver *driver;
>> void *driver_data;
>> struct ofono_atom *atom;
>> + command_handler_t handlers[256];
>
> There are only 41 proactive commands, and we'll implement a subset. This
> seems a bit excessive. However, if we can't come up with anything more
> clever, I'm actually fine with it for fast access.
Ok, changed it to command_handler_t
handlers[STK_COMMAND_TYPE_END_SESSION + 1]; (that equals 0x82), is
that ok?
>
>> + struct stk_command *pending_cmd;
>> };
>>
>> +static void stk_respond(struct ofono_stk *stk, struct stk_command *cmd,
>> + struct stk_response *rsp, generic_cb_t cb)
>> +{
>
> Can we avoid passing in the original stk_command object? Since only one
> command is pending at a time, sounds like we should be able to set it
> elsewhere easily.
Ok, I'll remove the parameter completely since it was only used twice
in the file.
>
> Is the plan to make this public API at some point? If so it definitely
> requires userdata.
I was rather wanting them to be local, other files wouldn't need to
deal with stk_response or stk_envelope at all, instead stk.c would
pick the important bits from the structure and perhaps call a function
in the other file.
>
>> + struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
>> + const guint8 *tlv;
>> + unsigned int tlv_len;
>> +
>> + if (cmd == NULL)
>> + cmd = stk->pending_cmd;
>> + else
>> + stk->pending_cmd = cmd;
>> +
>> + rsp->src = STK_DEVICE_IDENTITY_TYPE_TERMINAL;
>> + rsp->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
>> + rsp->number = cmd->number;
>> + rsp->type = cmd->type;
>> + rsp->qualifier = cmd->qualifier;
>> +
>> + if (stk->driver->terminal_response == NULL) {
>> + cb(&error, stk);
>> + return;
>> + }
>> +
>> + tlv = stk_pdu_from_response(rsp, &tlv_len);
>> + if (!tlv) {
>> + cb(&error, stk);
>> + return;
>> + }
>> +
>> + stk->driver->terminal_response(stk, tlv_len, tlv,
>> + (ofono_stk_generic_cb_t) cb, stk);
>> +}
>> +
>> +static void stk_send_envelope(struct ofono_stk *stk, struct stk_envelope
>> *e, + envelope_cb_t cb)
>
> Same comments here. Should this be public API with userdata?
>
>> +{
>> + struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
>> + const guint8 *tlv;
>> + unsigned int tlv_len;
>> +
>> + e->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
>> +
>> + if (stk->driver->envelope == NULL) {
>> + cb(&error, NULL, -1, stk);
>> + return;
>> + }
>> +
>> + tlv = stk_pdu_from_envelope(e, &tlv_len);
>> + if (!tlv) {
>> + cb(&error, NULL, -1, stk);
>> + return;
>> + }
>> +
>> + stk->driver->envelope(stk, tlv_len, tlv,
>> + (ofono_stk_envelope_cb_t) cb, stk);
>> +}
>> +
>> static void stk_cbs_download_cb(const struct ofono_error *error,
>> - const unsigned char *data, int len, void *user)
>> + const unsigned char *data, int len,
>> + struct ofono_stk *stk)
>> {
>> if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>> ofono_error("CellBroadcast download to UICC failed");
>> @@ -56,34 +126,47 @@ static void stk_cbs_download_cb(const struct
>> ofono_error *error, return;
>> }
>>
>> + if (len)
>> + ofono_error("CellBroadcast download returned %i bytes of data",
>> + len);
>> +
>> DBG("CellBroadcast download to UICC reported no error");
>> }
>>
>> void __ofono_cbs_sim_download(struct ofono_stk *stk, const struct cbs
>> *msg) {
>> - const guint8 *tlv;
>> - unsigned int tlv_len;
>> struct stk_envelope e;
>>
>> - if (stk->driver->envelope == NULL)
>> - return;
>> + memset(&e, 0, sizeof(e));
>>
>> e.type = STK_ENVELOPE_TYPE_CBS_PP_DOWNLOAD;
>> e.src = STK_DEVICE_IDENTITY_TYPE_NETWORK;
>> - e.dst = STK_DEVICE_IDENTITY_TYPE_UICC;
>> memcpy(&e.cbs_pp_download.page, msg, sizeof(msg));
>>
>> - tlv = stk_pdu_from_envelope(&e, &tlv_len);
>> - if (!tlv)
>> + stk_send_envelope(stk, &e, stk_cbs_download_cb);
>> +}
>> +
>> +static void stk_command_cb(const struct ofono_error *error,
>> + struct ofono_stk *stk)
>> +{
>> + stk_command_free(stk->pending_cmd);
>> + stk->pending_cmd = NULL;
>> +
>> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>> + ofono_error("TERMINAL RESPONSE to a UICC command failed");
>> + /* "The ME may retry to deliver the same Cell Broadcast
>> + * page." */
>> return;
>> + }
>>
>> - stk->driver->envelope(stk, tlv_len, tlv, stk_cbs_download_cb, stk);
>> + DBG("TERMINAL RESPONSE to a command reported no errors");
>> }
>>
>> void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
>> int length, const unsigned char *pdu)
>> {
>> struct stk_command *cmd;
>> + struct stk_response rsp;
>> char *buf;
>> int i;
>>
>> @@ -97,14 +180,49 @@ void ofono_stk_proactive_command_notify(struct
>> ofono_stk *stk, ofono_error("Can't parse proactive command: %s", buf);
>> g_free(buf);
>>
>> - /* TODO: return TERMINAL RESPONSE with permanent error */
>> + /*
>> + * There's nothing we can do, we'd need at least Command
>> + * Details to respond with an error.
>> + */
>> return;
>> }
>> + g_free(buf);
>>
>> - /* TODO: execute */
>> + memset(&rsp, 0, sizeof(rsp));
>>
>> - g_free(buf);
>> - stk_command_free(cmd);
>> + switch (cmd->status) {
>> + case STK_PARSE_RESULT_OK:
>> + if (stk->handlers[cmd->type]) {
>> + if (stk->handlers[cmd->type](cmd, &rsp, stk) == TRUE)
>> + break;
>> +
>> + stk->pending_cmd = cmd;
>> + return;
>> + }
>> + /* Fall through */
>> +
>> + case STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD:
>> + default:
>> + rsp.result.type = STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
>> + break;
>> +
>> + case STK_PARSE_RESULT_MISSING_VALUE:
>> + rsp.result.type = STK_RESULT_TYPE_MINIMUM_NOT_MET;
>> + break;
>> +
>> + case STK_PARSE_RESULT_DATA_NOT_UNDERSTOOD:
>> + rsp.result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
>> + break;
>> + }
>> +
>> + stk_respond(stk, cmd, &rsp, stk_command_cb);
>> +}
>> +
>> +static void stk_command_handler_register(struct ofono_stk *stk,
>> + enum stk_command_type type,
>> + command_handler_t handler)
>> +{
>> + stk->handlers[type] = handler;
>
> More error handling, might want to return an error if something is already
> registered.
Ok, added a check.
Regards
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-06-22 22:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-16 15:26 [PATCH 1/4][RFC] stkutil: Refactor command parser error handling Andrzej Zaborowski
2010-06-16 15:26 ` [PATCH 2/4] stk: Allow registering proactive command handlers Andrzej Zaborowski
2010-06-22 21:42 ` Denis Kenzior
2010-06-22 22:14 ` Andrzej Zaborowski
2010-06-16 15:26 ` [PATCH 3/4] stk: Handle the More Time command as a nop Andrzej Zaborowski
2010-06-16 15:26 ` [PATCH 4/4] Add STK Menu D-bus interface Andrzej Zaborowski
2010-06-22 21:44 ` [PATCH 1/4][RFC] stkutil: Refactor command parser error handling Denis Kenzior
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.