Hi Marius,
On 6/9/20 3:21 PM, Marius Gripsgard wrote:
This implements data capability bearer notify to qmi modem.
Since this is included in the serving system response this
just adds a new data extraction for dc.
---
drivers/qmimodem/gprs.c | 29 +++++++++++++++++++++++++++++
drivers/qmimodem/nas.c | 36 ++++++++++++++++++++++++++++++++++++
drivers/qmimodem/nas.h | 23 +++++++++++++++++++++++
3 files changed, 88 insertions(+)
Looks good, just couple of nitpicks:
diff --git a/drivers/qmimodem/gprs.c b/drivers/qmimodem/gprs.c
index 07adbe9a..0ba24da7 100644
--- a/drivers/qmimodem/gprs.c
+++ b/drivers/qmimodem/gprs.c
@@ -68,6 +68,29 @@ static bool extract_ss_info(struct qmi_result *result, int
*status, int *tech)
return true;
}
+static bool extract_dc_info(struct qmi_result *result, int *bearer_tech)
+{
+ const struct qmi_nas_data_capability *dc;
+ uint16_t len;
+ int i;
+
+ DBG("");
+
+ dc = qmi_result_get(result, QMI_NAS_RESULT_DATA_CAPABILIT_STATUS, &len);
Should this be CAPABILITY?
+ if (!dc)
+ return false;
+
+ *bearer_tech = -1;
+ for (i = 0; i < dc->cap_count; i++) {
+ DBG("radio tech in use %d", dc->cap[i]);
+
+ *bearer_tech = qmi_nas_cap_to_bearer_tech(dc->cap[i]);
I'm fine with this, but out of curiosity: can there be multiple of these? Are
they sorted somehow?
+ }
+
+ return true;
+}
+
+
No double empty lines please
static void get_lte_attach_param_cb(struct qmi_result *result, void
*user_data)
{
struct ofono_gprs *gprs = user_data;
@@ -188,6 +211,7 @@ static int handle_ss_info(struct qmi_result *result, struct
ofono_gprs *gprs)
struct gprs_data *data = ofono_gprs_get_data(gprs);
int status;
int tech;
+ int bearer_tech;
DBG("");
@@ -209,6 +233,11 @@ static int handle_ss_info(struct qmi_result *result, struct ofono_gprs *gprs)
data->last_auto_context_id = 0;
}
+ if (!extract_dc_info(result, &bearer_tech))
+ return -1;
So bearer_tech is pretty much optional, are you sure you want to return an error
here if the extraction fails? Maybe something like:
if (extract_dc_info(result, &bearer_tech)
ofono_grps_bearer_notify(gprs, bearer_tech);
would be a bit more conservative.
+
+ ofono_gprs_bearer_notify(gprs, bearer_tech);
+
return status;
}
Regards,
-Denis
_______________________________________________
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org