Hi Andrew, >> + >> + stk->tx_buffer.data.array = g_try_malloc(oc->buf_size); >> + if (stk->tx_buffer.data.array == NULL) { >> + rsp.result.type = STK_RESULT_TYPE_NOT_CAPABLE; >> + goto out; >> + } > > A minor nitpick: we tend to return one of the temporary error codes > when out of memory. > OK, I will return in this case the general result "STK_RESULT_TYPE_TERMINAL_BUSY" with the additional info "no specific cause". >> +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_TERMINATE: >> + send_simple_response(stk, STK_RESULT_TYPE_USER_TERMINATED); >> + return; >> + >> + case STK_AGENT_RESULT_TIMEOUT: >> + confirm = FALSE; >> + /* Fall through */ >> + >> + case STK_AGENT_RESULT_OK: >> + if (confirm) >> + break; >> + /* Fall through */ > > According to 6.4.27 we should return STK_RESULT_TYPE_USER_REJECT if > the user does not accept the channel setup. > >> + >> + default: >> + send_simple_response(stk, STK_RESULT_TYPE_TERMINAL_BUSY); >> + return; >> + } > > I think Jeevaka determined that this needs an additional info field. In practice, I applied the same code than "confirm_launch_browser_cb". But you're right, for Open channel, we should return the terminal response "User did not accept the proactive command". For the additional info field, I just simplified the code seen in "confirm_launch_browser_cb" as in this case, no specific cause is given. But your remark is true, it is mandatory to return an additional information in case of general result "STK_RESULT_TYPE_TERMINAL_BUSY". To clean both issues you raised and for an homogeneous behavior, I propose to follow the code written in "confirm_call_cb".Thus, in case of no answer, I will return STK_RESULT_TYPE_USER_TERMINATED and not STK_RESULT_TYPE_TERMINAL_BUSY. Thank you for your review. Best regards, Philippe.