| Message ID | 20241008091104.1567517-1-sean@geanix.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] network: allow status' and notifications on eutran networks | expand |
Hi Sean, On 10/8/24 4:11 AM, Sean Nyekjaer wrote: > SIMCom A7672E-FASE shows attached on LTE with +CREG <stat> either > 6 registered for "SMS only", home network (applicable only when E-UTRAN) > 7 registered for "SMS only", roaming (applicable only when <AcT> indicates E-UTRAN) > +COPS supplies the <AcT> = EUTRAN Do you know the distinction between registered/roaming and "SMS only" variants in practical terms? Are data calls available on "SMS only"? Voicecalls? > --- > src/network.c | 58 +++++++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 47 insertions(+), 11 deletions(-) > CI complains: Checkpatch Output ================= 0001-network-allow-status-and-notifications-on-eutran-net.patch:9: WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) 0001-network-allow-status-and-notifications-on-eutran-net.patch:36: ERROR: switch and case should be at the same indent 0001-network-allow-status-and-notifications-on-eutran-net.patch:63: ERROR: switch and case should be at the same indent 0001-network-allow-status-and-notifications-on-eutran-net.patch:92: ERROR: switch and case should be at the same indent 0001-network-allow-status-and-notifications-on-eutran-net.patch:114: ERROR: switch and case should be at the same indent total: 4 errors, 1 warnings, 100 lines checked 0002-gprs-allow-attached-updates-and-status-on-eutran-net.patch:9: WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) total: 0 errors, 1 warnings, 47 lines checked > diff --git a/src/network.c b/src/network.c > index 40626179..02853e23 100644 > --- a/src/network.c > +++ b/src/network.c > @@ -376,7 +376,8 @@ static char *get_operator_display_name(struct ofono_netreg *netreg) > return name; > } > > - if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED) > + if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED || > + netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN) > home_or_spdi = TRUE; > else > home_or_spdi = sim_spdi_lookup(netreg->spdi, > @@ -1205,9 +1206,15 @@ static void current_operator_callback(const struct ofono_error *error, > * in which case the operator information frequently comes in bogus. > * We ignore it here > */ > - if (netreg->status != NETWORK_REGISTRATION_STATUS_REGISTERED && > - netreg->status != NETWORK_REGISTRATION_STATUS_ROAMING) > - current = NULL; > + switch (netreg->status) { > + case NETWORK_REGISTRATION_STATUS_REGISTERED: > + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: > + case NETWORK_REGISTRATION_STATUS_ROAMING: > + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: > + break; > + default: > + current = NULL; > + } > > if (error->type != OFONO_ERROR_TYPE_NO_ERROR) { > DBG("Error during current operator"); > @@ -1324,6 +1331,8 @@ static void notify_emulator_status(struct ofono_atom *atom, void *data) > void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status, > int lac, int ci, int tech) > { > + ofono_bool_t netreg_status; > + > if (netreg == NULL) > return; > > @@ -1351,8 +1360,18 @@ void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status, > if (netreg->technology != tech) > set_registration_technology(netreg, tech); > > - if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED || > - netreg->status == NETWORK_REGISTRATION_STATUS_ROAMING) { > + switch (netreg->status) { > + case NETWORK_REGISTRATION_STATUS_REGISTERED: > + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: > + case NETWORK_REGISTRATION_STATUS_ROAMING: > + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: > + netreg_status = true; > + break; > + default: > + netreg_status = false; > + } > + > + if (netreg_status) { > if (netreg->driver->current_operator != NULL) > netreg->driver->current_operator(netreg, > current_operator_callback, netreg); > @@ -1448,6 +1467,7 @@ static void init_registration_status(const struct ofono_error *error, > void *data) > { > struct ofono_netreg *netreg = data; > + ofono_bool_t netreg_status; Simple bool is fine. The plan is to remove ofono_bool_t entirely. > > if (error->type != OFONO_ERROR_TYPE_NO_ERROR) { > DBG("Error during registration status query"); > @@ -1460,8 +1480,18 @@ static void init_registration_status(const struct ofono_error *error, > * Bootstrap our signal strength value without waiting for the > * stack to report it > */ > - if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED || > - netreg->status == NETWORK_REGISTRATION_STATUS_ROAMING) { > + switch (netreg->status) { > + case NETWORK_REGISTRATION_STATUS_REGISTERED: > + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: > + case NETWORK_REGISTRATION_STATUS_ROAMING: > + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: > + netreg_status = true; > + break; > + default: > + netreg_status = false; > + } > + > + if (netreg_status) { > if (netreg->driver->strength != NULL) > netreg->driver->strength(netreg, > signal_strength_callback, netreg); > @@ -1516,9 +1546,15 @@ void ofono_netreg_strength_notify(struct ofono_netreg *netreg, int strength) > * Theoretically we can get signal strength even when not registered > * to any network. However, what do we do with it in that case? > */ > - if (netreg->status != NETWORK_REGISTRATION_STATUS_REGISTERED && > - netreg->status != NETWORK_REGISTRATION_STATUS_ROAMING) > - return; > + switch (netreg->status) { > + case NETWORK_REGISTRATION_STATUS_REGISTERED: > + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: > + case NETWORK_REGISTRATION_STATUS_ROAMING: > + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: > + break; > + default: > + return; > + } Might be nicer to do: if (!L_IN_SET(netreg->status, NETWORK_REGISTRATION_STATUS_REGISTERED, NETWORK_REGISTRATION_STATUS_ROAMING, ...)) return; > > DBG("strength %d", strength); > Regards, -Denis
Hi Denis, On Wed, Oct 09, 2024 at 10:35:47AM +0100, Denis Kenzior wrote: > Hi Sean, > > On 10/8/24 4:11 AM, Sean Nyekjaer wrote: > > SIMCom A7672E-FASE shows attached on LTE with +CREG <stat> either > > 6 registered for "SMS only", home network (applicable only when E-UTRAN) > > 7 registered for "SMS only", roaming (applicable only when <AcT> indicates E-UTRAN) > > +COPS supplies the <AcT> = EUTRAN > > Do you know the distinction between registered/roaming and "SMS only" > variants in practical terms? Are data calls available on "SMS only"? > Voicecalls? To my limited knowledge; E-UTRAN(let us call it LTE) doesn't have voicecalls, that why we have VoLTE. With the SIMCom modem on LTE in NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN mode we have data and legacy sms available. > > > --- > > src/network.c | 58 +++++++++++++++++++++++++++++++++++++++++---------- > > 1 file changed, 47 insertions(+), 11 deletions(-) > > > > CI complains: > > Checkpatch Output > ================= > 0001-network-allow-status-and-notifications-on-eutran-net.patch:9: WARNING: > Prefer a maximum 75 chars per line (possible unwrapped commit description?) > 0001-network-allow-status-and-notifications-on-eutran-net.patch:36: ERROR: > switch and case should be at the same indent > 0001-network-allow-status-and-notifications-on-eutran-net.patch:63: ERROR: > switch and case should be at the same indent > 0001-network-allow-status-and-notifications-on-eutran-net.patch:92: ERROR: > switch and case should be at the same indent > 0001-network-allow-status-and-notifications-on-eutran-net.patch:114: ERROR: > switch and case should be at the same indent > total: 4 errors, 1 warnings, 100 lines checked > 0002-gprs-allow-attached-updates-and-status-on-eutran-net.patch:9: WARNING: > Prefer a maximum 75 chars per line (possible unwrapped commit description?) > total: 0 errors, 1 warnings, 47 lines checkeda Can I run checkpatch myself? > > > diff --git a/src/network.c b/src/network.c > > index 40626179..02853e23 100644 > > --- a/src/network.c > > +++ b/src/network.c > > @@ -376,7 +376,8 @@ static char *get_operator_display_name(struct ofono_netreg *netreg) > > return name; > > } > > - if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED) > > + if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED || > > + netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN) > > home_or_spdi = TRUE; > > else > > home_or_spdi = sim_spdi_lookup(netreg->spdi, > > @@ -1205,9 +1206,15 @@ static void current_operator_callback(const struct ofono_error *error, > > * in which case the operator information frequently comes in bogus. > > * We ignore it here > > */ > > - if (netreg->status != NETWORK_REGISTRATION_STATUS_REGISTERED && > > - netreg->status != NETWORK_REGISTRATION_STATUS_ROAMING) > > - current = NULL; > > + switch (netreg->status) { > > + case NETWORK_REGISTRATION_STATUS_REGISTERED: > > + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: > > + case NETWORK_REGISTRATION_STATUS_ROAMING: > > + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: > > + break; > > + default: > > + current = NULL; > > + } > > if (error->type != OFONO_ERROR_TYPE_NO_ERROR) { > > DBG("Error during current operator"); > > @@ -1324,6 +1331,8 @@ static void notify_emulator_status(struct ofono_atom *atom, void *data) > > void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status, > > int lac, int ci, int tech) > > { > > + ofono_bool_t netreg_status; > > + > > if (netreg == NULL) > > return; > > @@ -1351,8 +1360,18 @@ void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status, > > if (netreg->technology != tech) > > set_registration_technology(netreg, tech); > > - if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED || > > - netreg->status == NETWORK_REGISTRATION_STATUS_ROAMING) { > > + switch (netreg->status) { > > + case NETWORK_REGISTRATION_STATUS_REGISTERED: > > + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: > > + case NETWORK_REGISTRATION_STATUS_ROAMING: > > + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: > > + netreg_status = true; > > + break; > > + default: > > + netreg_status = false; > > + } > > + > > + if (netreg_status) { > > if (netreg->driver->current_operator != NULL) > > netreg->driver->current_operator(netreg, > > current_operator_callback, netreg); > > @@ -1448,6 +1467,7 @@ static void init_registration_status(const struct ofono_error *error, > > void *data) > > { > > struct ofono_netreg *netreg = data; > > + ofono_bool_t netreg_status; > > Simple bool is fine. The plan is to remove ofono_bool_t entirely. OK > > > if (error->type != OFONO_ERROR_TYPE_NO_ERROR) { > > DBG("Error during registration status query"); > > @@ -1460,8 +1480,18 @@ static void init_registration_status(const struct ofono_error *error, > > * Bootstrap our signal strength value without waiting for the > > * stack to report it > > */ > > - if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED || > > - netreg->status == NETWORK_REGISTRATION_STATUS_ROAMING) { > > + switch (netreg->status) { > > + case NETWORK_REGISTRATION_STATUS_REGISTERED: > > + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: > > + case NETWORK_REGISTRATION_STATUS_ROAMING: > > + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: > > + netreg_status = true; > > + break; > > + default: > > + netreg_status = false; > > + } > > + > > + if (netreg_status) { > > if (netreg->driver->strength != NULL) > > netreg->driver->strength(netreg, > > signal_strength_callback, netreg); > > @@ -1516,9 +1546,15 @@ void ofono_netreg_strength_notify(struct ofono_netreg *netreg, int strength) > > * Theoretically we can get signal strength even when not registered > > * to any network. However, what do we do with it in that case? > > */ > > - if (netreg->status != NETWORK_REGISTRATION_STATUS_REGISTERED && > > - netreg->status != NETWORK_REGISTRATION_STATUS_ROAMING) > > - return; > > + switch (netreg->status) { > > + case NETWORK_REGISTRATION_STATUS_REGISTERED: > > + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: > > + case NETWORK_REGISTRATION_STATUS_ROAMING: > > + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: > > + break; > > + default: > > + return; > > + } > > Might be nicer to do: > > if (!L_IN_SET(netreg->status, NETWORK_REGISTRATION_STATUS_REGISTERED, > NETWORK_REGISTRATION_STATUS_ROAMING, > ...)) > return; Will do that for V2. > > > DBG("strength %d", strength); > > Regards, > -Denis /Sean
diff --git a/src/network.c b/src/network.c index 40626179..02853e23 100644 --- a/src/network.c +++ b/src/network.c @@ -376,7 +376,8 @@ static char *get_operator_display_name(struct ofono_netreg *netreg) return name; } - if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED) + if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED || + netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN) home_or_spdi = TRUE; else home_or_spdi = sim_spdi_lookup(netreg->spdi, @@ -1205,9 +1206,15 @@ static void current_operator_callback(const struct ofono_error *error, * in which case the operator information frequently comes in bogus. * We ignore it here */ - if (netreg->status != NETWORK_REGISTRATION_STATUS_REGISTERED && - netreg->status != NETWORK_REGISTRATION_STATUS_ROAMING) - current = NULL; + switch (netreg->status) { + case NETWORK_REGISTRATION_STATUS_REGISTERED: + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: + case NETWORK_REGISTRATION_STATUS_ROAMING: + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: + break; + default: + current = NULL; + } if (error->type != OFONO_ERROR_TYPE_NO_ERROR) { DBG("Error during current operator"); @@ -1324,6 +1331,8 @@ static void notify_emulator_status(struct ofono_atom *atom, void *data) void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status, int lac, int ci, int tech) { + ofono_bool_t netreg_status; + if (netreg == NULL) return; @@ -1351,8 +1360,18 @@ void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status, if (netreg->technology != tech) set_registration_technology(netreg, tech); - if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED || - netreg->status == NETWORK_REGISTRATION_STATUS_ROAMING) { + switch (netreg->status) { + case NETWORK_REGISTRATION_STATUS_REGISTERED: + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: + case NETWORK_REGISTRATION_STATUS_ROAMING: + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: + netreg_status = true; + break; + default: + netreg_status = false; + } + + if (netreg_status) { if (netreg->driver->current_operator != NULL) netreg->driver->current_operator(netreg, current_operator_callback, netreg); @@ -1448,6 +1467,7 @@ static void init_registration_status(const struct ofono_error *error, void *data) { struct ofono_netreg *netreg = data; + ofono_bool_t netreg_status; if (error->type != OFONO_ERROR_TYPE_NO_ERROR) { DBG("Error during registration status query"); @@ -1460,8 +1480,18 @@ static void init_registration_status(const struct ofono_error *error, * Bootstrap our signal strength value without waiting for the * stack to report it */ - if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED || - netreg->status == NETWORK_REGISTRATION_STATUS_ROAMING) { + switch (netreg->status) { + case NETWORK_REGISTRATION_STATUS_REGISTERED: + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: + case NETWORK_REGISTRATION_STATUS_ROAMING: + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: + netreg_status = true; + break; + default: + netreg_status = false; + } + + if (netreg_status) { if (netreg->driver->strength != NULL) netreg->driver->strength(netreg, signal_strength_callback, netreg); @@ -1516,9 +1546,15 @@ void ofono_netreg_strength_notify(struct ofono_netreg *netreg, int strength) * Theoretically we can get signal strength even when not registered * to any network. However, what do we do with it in that case? */ - if (netreg->status != NETWORK_REGISTRATION_STATUS_REGISTERED && - netreg->status != NETWORK_REGISTRATION_STATUS_ROAMING) - return; + switch (netreg->status) { + case NETWORK_REGISTRATION_STATUS_REGISTERED: + case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN: + case NETWORK_REGISTRATION_STATUS_ROAMING: + case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN: + break; + default: + return; + } DBG("strength %d", strength);