Hi Andrew,
> @@ -43,10 +43,61 @@ struct ofono_stk {
> const struct ofono_stk_driver *driver;
> void *driver_data;
> struct ofono_atom *atom;
> + struct stk_command *pending_cmd;
> + void (*cancel_cmd)(struct ofono_stk *stk);
I notice that you remove cancel_cmd in a later patch. Please don't do
that. Clean this patch up not to include cancel_cmd and resubmit.
> };
>
> +static int stk_respond(struct ofono_stk *stk, struct stk_response *rsp,
> + void (*cb)(const struct ofono_error *error,
> + struct ofono_stk *stk))
Please don't do this. The callback should just take the standard
ofono_stk_generic_cb_t
> +{
> + const guint8 *tlv;
> + unsigned int tlv_len;
> +
> + rsp->src = STK_DEVICE_IDENTITY_TYPE_TERMINAL;
> + rsp->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
> + rsp->number = stk->pending_cmd->number;
> + rsp->type = stk->pending_cmd->type;
> + rsp->qualifier = stk->pending_cmd->qualifier;
> +
> + if (stk->driver->terminal_response == NULL)
> + return -ENOSYS;
Might want to do this check first.
> +
> + tlv = stk_pdu_from_response(rsp, &tlv_len);
> + if (!tlv)
> + return -EINVAL;
> +
> + stk->driver->terminal_response(stk, tlv_len, tlv,
> + (ofono_stk_generic_cb_t) cb, stk);
Do not cast please, just bite the bullet and use already defined
callback typedefs and use
ofono_stk *stk = user;
> + return 0;
Empty line before return 0 please.
> +}
> +
> +static int stk_send_envelope(struct ofono_stk *stk, struct stk_envelope *e,
> + void (*cb)(const struct ofono_error *error,
> + const unsigned char *data,
> + int length,
> + struct ofono_stk *stk))
> +{
> + const guint8 *tlv;
> + unsigned int tlv_len;
> +
> + e->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
> +
> + if (stk->driver->envelope == NULL)
> + return -ENOSYS;
> +
> + tlv = stk_pdu_from_envelope(e, &tlv_len);
> + if (!tlv)
> + return -EINVAL;
> +
> + stk->driver->envelope(stk, tlv_len, tlv,
> + (ofono_stk_envelope_cb_t) cb, stk);
> + return 0;
Same comment as above, and also an empty line before return 0 please.
> +}
> +
> 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");
> @@ -55,36 +106,62 @@ 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 ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
> struct stk_envelope e;
> + int err;
>
> - 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)
> + err = stk_send_envelope(stk, &e, stk_cbs_download_cb);
> + if (err)
> + stk_cbs_download_cb(&error, NULL, -1, stk);
> +}
> +
> +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_cancel(struct ofono_stk *stk)
> +{
> + if (!stk->pending_cmd)
> + return;
> +
> + stk->cancel_cmd(stk);
I notice this code has also been removed in future patches. Please
clean up this patch.
> }
>
> void ofono_stk_proactive_command_notify(struct ofono_stk *stk,
> int length, const unsigned char *pdu)
> {
> - struct stk_command *cmd;
> + struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
> + struct stk_response rsp;
> char *buf;
> - int i;
> + int i, err;
> + gboolean respond = TRUE;
>
> buf = g_try_malloc(length * 2 + 1);
> if (!buf)
> @@ -93,18 +170,51 @@ void ofono_stk_proactive_command_notify(struct ofono_stk
> *stk,
> for (i = 0; i < length; i ++)
> sprintf(buf + i * 2, "%02hhx", pdu[i]);
>
> - cmd = stk_command_new_from_pdu(pdu, length);
> - if (!cmd) {
> + stk->pending_cmd = stk_command_new_from_pdu(pdu, length);
> + if (!stk->pending_cmd) {
> ofono_error("Can't parse proactive command: %s", buf);
>
> - /* TODO: return TERMINAL RESPONSE with permanent error */
> + /*
> + * Nothing we can do, we'd need at least Command Details
> + * to be able to respond with an error.
> + */
> goto done;
> }
>
> - /* TODO: execute */
> - ofono_info("Proactive command PDU: %s", buf);
> + ofono_debug("Proactive command PDU: %s", buf);
> +
> + memset(&rsp, 0, sizeof(rsp));
> +
> + switch (stk->pending_cmd->status) {
> + case STK_PARSE_RESULT_OK:
> + switch (stk->pending_cmd->type) {
> + default:
> + rsp.result.type =
> + STK_RESULT_TYPE_COMMAND_NOT_UNDERSTOOD;
> + break;
> + }
> +
> + if (respond)
> + break;
> + return;
> +
> + case STK_PARSE_RESULT_TYPE_NOT_UNDERSTOOD:
> + default:
Does this even work? You might want to move this down to the end of the
switch/case.
> + 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_command_free(cmd);
> + err = stk_respond(stk, &rsp, stk_command_cb);
> + if (err)
> + stk_command_cb(&error, stk);
>
> done:
> g_free(buf);
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
