10:46 a.m.
This patch series add the ModemTechnologies property to the
RadioSettings interface. Its purpose is to show the list of valid
values for the TechnologyPreference property in the same interface.
The use case is letting a UI show the possible RATs so the user cannot
select an invalid value for a given modem.
Alfonso Sanchez-Beato (4):
doc: Add ModemTechnologies property
include: Add method to list RATs to radio-settings
src: Implement RAT list property
test: Add ModemTechnologies to list-modems
doc/radio-settings-api.txt | 5 ++++
include/radio-settings.h | 11 +++++++++
src/radio-settings.c | 61 +++++++++++++++++++++++++++++++++++++++++++++-
test/list-modems | 3 ++-
4 files changed, 78 insertions(+), 2 deletions(-)
--
2.1.0
10:46 a.m.
New subject: [PATCH 1/4] doc: Add ModemTechnologies property
This new property in org.ofono.RadioSettings interface will hold the
possible values for radio access technology for the modem.
---
doc/radio-settings-api.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/doc/radio-settings-api.txt b/doc/radio-settings-api.txt
index 9f9256f..cbeac3b 100644
--- a/doc/radio-settings-api.txt
+++ b/doc/radio-settings-api.txt
@@ -45,6 +45,11 @@ Properties string TechnologyPreference [readwrite]
"umts" Only UMTS used for radio access.
"lte" Only LTE used for radio access.
+ array{string} ModemTechnologies [readonly, optional]
+
+ List of values for TechnologyPreference property
+ supported by the modem.
+
string GsmBand [readwrite, optional]
Frequency band in which the modem is allowed to
--
2.1.0
10:11 p.m.
New subject: [PATCH 1/4] doc: Add ModemTechnologies property
Hi Alfonso,
On 12/05/2014 10:46 AM, Alfonso Sanchez-Beato wrote:
This new property in org.ofono.RadioSettings interface will hold the
possible values for radio access technology for the modem.
---
doc/radio-settings-api.txt | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/doc/radio-settings-api.txt b/doc/radio-settings-api.txt
index 9f9256f..cbeac3b 100644
--- a/doc/radio-settings-api.txt
+++ b/doc/radio-settings-api.txt
@@ -45,6 +45,11 @@ Properties string TechnologyPreference [readwrite]
"umts" Only UMTS used for radio access.
"lte" Only LTE used for radio access.
+ array{string} ModemTechnologies [readonly, optional]
+
Lets name this AvailableTechnologies
+ List of values for TechnologyPreference property
+ supported by the modem.
+
string GsmBand [readwrite, optional]
Frequency band in which the modem is allowed to
Regards,
-Denis
10:46 a.m.
New subject: [PATCH 2/4] include: Add method to list RATs to radio-settings
---
include/radio-settings.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/radio-settings.h b/include/radio-settings.h
index 1d0ca3e..d7481b3 100644
--- a/include/radio-settings.h
+++ b/include/radio-settings.h
@@ -35,6 +35,9 @@ enum ofono_radio_access_mode {
OFONO_RADIO_ACCESS_MODE_LTE = 3,
};
+/* Set this to latest in ofono_radio_access_mode + 1 */
+#define OFONO_RADIO_ACCESS_MODE_LAST (OFONO_RADIO_ACCESS_MODE_LTE + 1)
+
enum ofono_radio_band_gsm {
OFONO_RADIO_BAND_GSM_ANY,
OFONO_RADIO_BAND_GSM_850,
@@ -80,6 +83,11 @@ typedef void (*ofono_radio_settings_fast_dormancy_query_cb_t)(
ofono_bool_t enable,
void *data);
+typedef void (*ofono_radio_settings_modem_rats_query_cb_t)(
+ const struct ofono_error *error,
+ const ofono_bool_t rats[],
+ void *data);
+
struct ofono_radio_settings_driver {
const char *name;
int (*probe)(struct ofono_radio_settings *rs, unsigned int vendor,
@@ -107,6 +115,9 @@ struct ofono_radio_settings_driver {
ofono_bool_t enable,
ofono_radio_settings_fast_dormancy_set_cb_t,
void *data);
+ void (*query_modem_rats)(struct ofono_radio_settings *rs,
+ ofono_radio_settings_modem_rats_query_cb_t cb,
+ void *data);
};
int ofono_radio_settings_driver_register(
--
2.1.0
10:24 p.m.
New subject: [PATCH 2/4] include: Add method to list RATs to radio-settings
Hi Alfonso,
On 12/05/2014 10:46 AM, Alfonso Sanchez-Beato wrote:
---
include/radio-settings.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/radio-settings.h b/include/radio-settings.h
index 1d0ca3e..d7481b3 100644
--- a/include/radio-settings.h
+++ b/include/radio-settings.h
@@ -35,6 +35,9 @@ enum ofono_radio_access_mode {
OFONO_RADIO_ACCESS_MODE_LTE = 3,
};
+/* Set this to latest in ofono_radio_access_mode + 1 */
+#define OFONO_RADIO_ACCESS_MODE_LAST (OFONO_RADIO_ACCESS_MODE_LTE + 1)
+
I would actually change radio_access_mode into a bit field, that way the
available access modes can be passed as an unsigned integer.
e.g. something like:
OFONO_RADIO_ACCESS_MODE_ANY = 0,
OFONO_RADIO_ACCESS_MODE_GSM = 0x1,
OFONO_RADIO_ACCESS_MODE_UMTS = 0x2,
OFONO_RADIO_ACCESS_MODE_LTE = 0x4,
enum ofono_radio_band_gsm {
OFONO_RADIO_BAND_GSM_ANY,
OFONO_RADIO_BAND_GSM_850,
@@ -80,6 +83,11 @@ typedef void (*ofono_radio_settings_fast_dormancy_query_cb_t)(
ofono_bool_t enable,
void *data);
+typedef void (*ofono_radio_settings_modem_rats_query_cb_t)(
+ const struct ofono_error *error,
+ const ofono_bool_t rats[],
+ void *data);
+
Then this becomes:
typedef void (*ofono_radio_settings_available_rats_query_cb_t)(
const struct ofono_error *error,
unsigned int available_rats,
void *data);
struct ofono_radio_settings_driver {
const char *name;
int (*probe)(struct ofono_radio_settings *rs, unsigned int vendor,
@@ -107,6 +115,9 @@ struct ofono_radio_settings_driver {
ofono_bool_t enable,
ofono_radio_settings_fast_dormancy_set_cb_t,
void *data);
+ void (*query_modem_rats)(struct ofono_radio_settings *rs,
+ ofono_radio_settings_modem_rats_query_cb_t cb,
+ void *data);
name this query_available_rats please.
};
int ofono_radio_settings_driver_register(
Regards,
-Denis
10:46 a.m.
New subject: [PATCH 3/4] src: Implement RAT list property
---
src/radio-settings.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/src/radio-settings.c b/src/radio-settings.c
index d1b1cc1..4967994 100644
--- a/src/radio-settings.c
+++ b/src/radio-settings.c
@@ -48,6 +48,8 @@ struct ofono_radio_settings {
enum ofono_radio_band_gsm pending_band_gsm;
enum ofono_radio_band_umts pending_band_umts;
ofono_bool_t fast_dormancy_pending;
+ ofono_bool_t modem_rats[OFONO_RADIO_ACCESS_MODE_LAST];
+ ofono_bool_t modem_rats_filled;
const struct ofono_radio_settings_driver *driver;
void *driver_data;
struct ofono_atom *atom;
@@ -222,6 +224,21 @@ static DBusMessage *radio_get_properties_reply(DBusMessage *msg,
DBUS_TYPE_BOOLEAN, &value);
}
+ if (rs->driver->query_modem_rats) {
+ const char *rats_strs[OFONO_RADIO_ACCESS_MODE_LAST + 1];
+ const char *(*strs)[] = &rats_strs;
+ int i, str_i;
+
+ for (i = 0, str_i = 0; i < OFONO_RADIO_ACCESS_MODE_LAST; ++i)
+ if (rs->modem_rats[i])
+ rats_strs[str_i++] =
+ radio_access_mode_to_string(i);
+ rats_strs[str_i] = NULL;
+
+ ofono_dbus_dict_append_array(&dict, "ModemTechnologies",
+ DBUS_TYPE_STRING, &strs);
+ }
+
dbus_message_iter_close_container(&iter, &dict);
return reply;
@@ -374,6 +391,43 @@ static void radio_send_properties_reply(struct ofono_radio_settings
*rs)
__ofono_dbus_pending_reply(&rs->pending, reply);
}
+static void radio_set_modem_rats(struct ofono_radio_settings *rs,
+ const ofono_bool_t rats[])
+{
+ memcpy(rs->modem_rats, rats, sizeof(rs->modem_rats));
+ rs->modem_rats_filled = TRUE;
+}
+
+static void radio_modem_rats_query_callback(const struct ofono_error *error,
+ const ofono_bool_t rats[],
+ void *data)
+{
+ struct ofono_radio_settings *rs = data;
+ DBusMessage *reply;
+
+ if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+ DBG("Error during modem rats query");
+
+ reply = __ofono_error_failed(rs->pending);
+ __ofono_dbus_pending_reply(&rs->pending, reply);
+
+ return;
+ }
+
+ radio_set_modem_rats(rs, rats);
+ radio_send_properties_reply(rs);
+}
+
+static void radio_query_modem_rats(struct ofono_radio_settings *rs)
+{
+ if (rs->driver->query_modem_rats == NULL) {
+ radio_send_properties_reply(rs);
+ return;
+ }
+
+ rs->driver->query_modem_rats(rs, radio_modem_rats_query_callback, rs);
+}
+
static void radio_fast_dormancy_query_callback(const struct ofono_error *error,
ofono_bool_t enable, void *data)
{
@@ -390,7 +444,12 @@ static void radio_fast_dormancy_query_callback(const struct
ofono_error *error,
}
radio_set_fast_dormancy(rs, enable);
- radio_send_properties_reply(rs);
+
+ /* Modem technology is not supposed to change, so one query is enough */
+ if (rs->modem_rats_filled)
+ radio_send_properties_reply(rs);
+ else
+ radio_query_modem_rats(rs);
}
static void radio_query_fast_dormancy(struct ofono_radio_settings *rs)
--
2.1.0
10:46 p.m.
New subject: [PATCH 3/4] src: Implement RAT list property
Hi Alfonso,
On 12/05/2014 10:46 AM, Alfonso Sanchez-Beato wrote:
---
src/radio-settings.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 60 insertions(+), 1 deletion(-)
diff --git a/src/radio-settings.c b/src/radio-settings.c
index d1b1cc1..4967994 100644
--- a/src/radio-settings.c
+++ b/src/radio-settings.c
@@ -48,6 +48,8 @@ struct ofono_radio_settings {
enum ofono_radio_band_gsm pending_band_gsm;
enum ofono_radio_band_umts pending_band_umts;
ofono_bool_t fast_dormancy_pending;
+ ofono_bool_t modem_rats[OFONO_RADIO_ACCESS_MODE_LAST];
+ ofono_bool_t modem_rats_filled;
In general we use uint32_t flags and set the appropriate flag. See e.g.
src/voicecall.c for an example.
I think if you take my suggestions from patch 2, then a simple uint32_t
available_rats variable should be sufficient. If it is set to zero,
then the AvailableTechnologies property should be hidden.
const struct ofono_radio_settings_driver *driver;
void *driver_data;
struct ofono_atom *atom;
@@ -222,6 +224,21 @@ static DBusMessage *radio_get_properties_reply(DBusMessage *msg,
DBUS_TYPE_BOOLEAN, &value);
}
+ if (rs->driver->query_modem_rats) {
+ const char *rats_strs[OFONO_RADIO_ACCESS_MODE_LAST + 1];
+ const char *(*strs)[] = &rats_strs;
I'm seriously lost what you're trying to do here. &rats_strs is a no-op.
I suspect that const char **strs = rats_strs; would be a bit easier to
understand.
+ int i, str_i;
+
+ for (i = 0, str_i = 0; i < OFONO_RADIO_ACCESS_MODE_LAST; ++i)
+ if (rs->modem_rats[i])
+ rats_strs[str_i++] =
+ radio_access_mode_to_string(i);
+ rats_strs[str_i] = NULL;
+
+ ofono_dbus_dict_append_array(&dict, "ModemTechnologies",
+ DBUS_TYPE_STRING, &strs);
Please make this into AvailableTechnologies
+ }
+
dbus_message_iter_close_container(&iter, &dict);
return reply;
@@ -374,6 +391,43 @@ static void radio_send_properties_reply(struct ofono_radio_settings
*rs)
__ofono_dbus_pending_reply(&rs->pending, reply);
}
+static void radio_set_modem_rats(struct ofono_radio_settings *rs,
+ const ofono_bool_t rats[])
+{
+ memcpy(rs->modem_rats, rats, sizeof(rs->modem_rats));
+ rs->modem_rats_filled = TRUE;
A two line function that is called from only one place is not a good idea.
+}
+
+static void radio_modem_rats_query_callback(const struct ofono_error *error,
+ const ofono_bool_t rats[],
+ void *data)
+{
+ struct ofono_radio_settings *rs = data;
+ DBusMessage *reply;
+
+ if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+ DBG("Error during modem rats query");
+
+ reply = __ofono_error_failed(rs->pending);
+ __ofono_dbus_pending_reply(&rs->pending, reply);
+
+ return;
If possible the GetProperties call should not return an error. In this
particular case, if the query fails, simply omit AvailableTechnologies
from the properties dictionary. Hopefully the query succeeds next time.
+ }
+
+ radio_set_modem_rats(rs, rats);
+ radio_send_properties_reply(rs);
+}
+
+static void radio_query_modem_rats(struct ofono_radio_settings *rs)
+{
+ if (rs->driver->query_modem_rats == NULL) {
+ radio_send_properties_reply(rs);
+ return;
+ }
+
+ rs->driver->query_modem_rats(rs, radio_modem_rats_query_callback, rs);
+}
+
static void radio_fast_dormancy_query_callback(const struct ofono_error *error,
ofono_bool_t enable, void *data)
{
@@ -390,7 +444,12 @@ static void radio_fast_dormancy_query_callback(const struct
ofono_error *error,
}
radio_set_fast_dormancy(rs, enable);
- radio_send_properties_reply(rs);
+
+ /* Modem technology is not supposed to change, so one query is enough */
+ if (rs->modem_rats_filled)
+ radio_send_properties_reply(rs);
+ else
+ radio_query_modem_rats(rs);
}
static void radio_query_fast_dormancy(struct ofono_radio_settings *rs)
Regards,
-Denis
10:46 a.m.
New subject: [PATCH 4/4] test: Add ModemTechnologies to list-modems
---
test/list-modems | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/test/list-modems b/test/list-modems
index b9f510a..cc150f6 100755
--- a/test/list-modems
+++ b/test/list-modems
@@ -40,7 +40,8 @@ for path, properties in modems:
"PreferredLanguages",
"PrimaryContexts",
"LockedPins",
- "Features"]:
+ "Features",
+ "ModemTechnologies"]:
val = ""
for i in properties[key]:
val += i + " "
--
2.1.0
10:47 p.m.
New subject: [PATCH 4/4] test: Add ModemTechnologies to list-modems
Hi Alfonso,
On 12/05/2014 10:46 AM, Alfonso Sanchez-Beato wrote:
---
test/list-modems | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/test/list-modems b/test/list-modems
index b9f510a..cc150f6 100755
--- a/test/list-modems
+++ b/test/list-modems
@@ -40,7 +40,8 @@ for path, properties in modems:
"PreferredLanguages",
"PrimaryContexts",
"LockedPins",
- "Features"]:
+ "Features",
+ "ModemTechnologies"]:
AvailableTechnologies here please
val = ""
for i in properties[key]:
val += i + " "
Regards,
-Denis
10:51 p.m.
Hi Alfonso,
On 12/05/2014 10:46 AM, Alfonso Sanchez-Beato wrote:
This patch series add the ModemTechnologies property to the
RadioSettings interface. Its purpose is to show the list of valid
values for the TechnologyPreference property in the same interface.
The use case is letting a UI show the possible RATs so the user cannot
select an invalid value for a given modem.
Use case makes sense to me at least. Thanks for doing this.
Regards,
-Denis
6:34 a.m.
On Mon, Dec 8, 2014 at 5:51 AM, Denis Kenzior <denkenz(a)gmail.com> wrote:
Hi Alfonso,
On 12/05/2014 10:46 AM, Alfonso Sanchez-Beato wrote:
> This patch series add the ModemTechnologies property to the
> RadioSettings interface. Its purpose is to show the list of valid
> values for the TechnologyPreference property in the same interface.
> The use case is letting a UI show the possible RATs so the user cannot
> select an invalid value for a given modem.
>
>
Use case makes sense to me at least. Thanks for doing this.
Regards,
-Denis
Thanks for the detailed review. Sending the new patch set right now...
Br,
Alfonso
2241
days inactive
2245
days old
10 comments
2 participants
participants (2)
-
Alfonso Sanchez-Beato -
Denis Kenzior