Re: [PATCH 3/3] gprs.c: add list contexts for emulator
Hi Olivier, Gentle reminder not to top-post on this mailing list. On 03/22/2011 05:49 AM, Olivier Guiter wrote: Thanks Denis for your feedback. The idea behind this code is to propose the complete support of the usual AT+CGDCONT in the emulator. I think the best solution might be to stick on contexts exposed over DBus, but this could lead to problems like : a context is delete throught the DBus interface... how to handle this change in the AT emulator, and avoid the use of the deleted context ? Personally I'm not convinced CGDCONT management is even a good idea. The typical usecase is for DUN clients to send a CGDCONT string, but this can simply be accepted and ignored (for instance). Just overwriting oFono's context store is not a good idea anyway as you lose certain valuable information. For instance whether this context is used for internet (and ConnMan should pay attention to it) or it is used for mms, etc. It is also possible for the dial up client to provide context details that are identical to a context already defined in oFono. In which case those details should be used instead (for the reasons outlined in the previous paragraph) Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 3/3] gprs.c: add list contexts for emulator
Hi Denis, Thanks Denis for your feedback. The idea behind this code is to propose the complete support of the usual AT+CGDCONT in the emulator. I think the best solution might be to stick on contexts exposed over DBus, but this could lead to problems like : a context is delete throught the DBus interface... how to handle this change in the AT emulator, and avoid the use of the deleted context ? Personally I'm not convinced CGDCONT management is even a good idea. The typical usecase is for DUN clients to send a CGDCONT string, but this can simply be accepted and ignored (for instance). We are not looking at DUN only, but to the 'global' AT Emulator, that will be used during GCF/PTCRB certification. During certification, a lot of test require to set some GPRS context with specific parameters. And this cannot go 'directly' to the modem, without going through AP, because the test would not be relevant, or worse will failed, due to disconnect between oFono and modem. Just overwriting oFono's context store is not a good idea anyway as you lose certain valuable information. For instance whether this context is used for internet (and ConnMan should pay attention to it) or it is used for mms, etc. It is also possible for the dial up client to provide context details that are identical to a context already defined in oFono. In which case those details should be used instead (for the reasons outlined in the previous paragraph) So from my point of view, all context must be in sync, in oFono side and modem side. Regards, Bertrand. - Intel Corporation SAS (French simplified joint stock company) Registered headquarters: Les Montalets- 2, rue de Paris, 92196 Meudon Cedex, France Registration Number: 302 456 199 R.C.S. NANTERRE Capital: 4,572,000 Euros This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/3] gprs.c: add list contexts for emulator
Hi Bertrand, On 03/22/2011 10:17 AM, Aygon, Bertrand wrote: Hi Denis, Thanks Denis for your feedback. The idea behind this code is to propose the complete support of the usual AT+CGDCONT in the emulator. I think the best solution might be to stick on contexts exposed over DBus, but this could lead to problems like : a context is delete throught the DBus interface... how to handle this change in the AT emulator, and avoid the use of the deleted context ? Personally I'm not convinced CGDCONT management is even a good idea. The typical usecase is for DUN clients to send a CGDCONT string, but this can simply be accepted and ignored (for instance). We are not looking at DUN only, but to the 'global' AT Emulator, that will be used during GCF/PTCRB certification. I know exactly what you want this for. During certification, a lot of test require to set some GPRS context with specific parameters. And this cannot go 'directly' to the modem, without going through AP, because the test would not be relevant, or worse will failed, due to disconnect between oFono and modem. And I know this as well ;) However, I'm still not convinced. oFono simply works differently and you will encounter too many problems doing it this way. I listed some of them out already... Just overwriting oFono's context store is not a good idea anyway as you lose certain valuable information. For instance whether this context is used for internet (and ConnMan should pay attention to it) or it is used for mms, etc. It is also possible for the dial up client to provide context details that are identical to a context already defined in oFono. In which case those details should be used instead (for the reasons outlined in the previous paragraph) So from my point of view, all context must be in sync, in oFono side and modem side. The reality is doing so is pretty much a bad idea from a technical point of view. So sounds like you're in for some fun ;) Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH 3/3] gprs.c: add list contexts for emulator
---
src/gprs.c | 26 --
1 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/gprs.c b/src/gprs.c
index 04432c3..06d52f3 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -2863,11 +2863,31 @@ static void provision_contexts(struct ofono_gprs *gprs,
const char *mcc,
__ofono_gprs_provision_free_settings(settings, count);
}
+static void ofono_gprs_list_contexts(struct ofono_emulator *em, void *userdata)
+{
+ struct ofono_gprs *gprs = userdata;
+ GSList *l;
+ char buf[256];
+
+ struct pri_context *ctx;
+
+ for (l = gprs-contexts; l; l = l-next) {
+ ctx = l-data;
+
+ snprintf(buf, 255, +CGDCONT: %d,\%s\,\%s\,
+ ctx-id,
gprs_proto_to_string(ctx-context.proto),
+ ctx-context.apn);
+ ofono_emulator_send_info(em, buf, FALSE);
+ }
+}
+
/* Process the usual AT+CGDCONT command
*/
static void cgdcont_cb(struct ofono_emulator *em,
struct ofono_emulator_request *req, void *userdata)
{
+ struct ofono_gprs *gprs = userdata;
+ struct idmap *cid = gprs-cid_map;
struct ofono_error result;
char buf[256];
@@ -2876,7 +2896,8 @@ static void cgdcont_cb(struct ofono_emulator *em,
switch (ofono_emulator_request_get_type(req)) {
case OFONO_EMULATOR_REQUEST_TYPE_SUPPORT:
/* TODO: check additionnal parameters */
- snprintf(buf, 255, +CGDCONT:
(1-2),\IP\,,,(0-2),(0,1,2,3,4));
+ snprintf(buf, 255, +CGDCONT:
(1-%d),\IP\,,,(0-2),(0,1,2,3,4),
+ idmap_get_size(cid));
ofono_emulator_send_info(em, buf, FALSE);
result.type = OFONO_ERROR_TYPE_NO_ERROR;
ofono_emulator_send_final(em, result);
@@ -2888,7 +2909,8 @@ static void cgdcont_cb(struct ofono_emulator *em,
break;
case OFONO_EMULATOR_REQUEST_TYPE_QUERY:
- result.type = OFONO_ERROR_TYPE_FAILURE;
+ ofono_gprs_list_contexts(em, gprs);
+ result.type = OFONO_ERROR_TYPE_NO_ERROR;
ofono_emulator_send_final(em, result);
break;
case OFONO_EMULATOR_REQUEST_TYPE_SET:
--
1.7.1
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 3/3] gprs.c: add list contexts for emulator
Hi Olivier,
On 03/21/2011 08:45 AM, Olivier Guiter wrote:
---
src/gprs.c | 26 --
1 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/gprs.c b/src/gprs.c
index 04432c3..06d52f3 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -2863,11 +2863,31 @@ static void provision_contexts(struct ofono_gprs
*gprs, const char *mcc,
__ofono_gprs_provision_free_settings(settings, count);
}
+static void ofono_gprs_list_contexts(struct ofono_emulator *em, void
*userdata)
+{
+ struct ofono_gprs *gprs = userdata;
+ GSList *l;
+ char buf[256];
+
+ struct pri_context *ctx;
+
+ for (l = gprs-contexts; l; l = l-next) {
+ ctx = l-data;
+
+ snprintf(buf, 255, +CGDCONT: %d,\%s\,\%s\,
+ ctx-id,
gprs_proto_to_string(ctx-context.proto),
+ ctx-context.apn);
So you're mixing concepts here a bit.
gprs.c has two maps:
- pid_map
- cid_map
and two ids:
- pri_context::id
- ofono_gprs_primary_context::cid
Roughly speaking the cid_map lets us pick a context id that maps 1:1 to
the modem driver. Hence the use of ofono_gprs_set_cid_range. Basically
the driver tells us the valid range of context ids and oFono picks the
rest. For an AT modem the driver can simply use the cid picked by oFono
in activate_primary callback.
Internally oFono has another idmap for tracking unique ids for contexts
and ensuring some sane limit on their number. This is managed by
pid_map and pri_context::id.
The code you have right now would use the pid_map ids...
+ ofono_emulator_send_info(em, buf, FALSE);
+ }
+}
+
/* Process the usual AT+CGDCONT command
*/
static void cgdcont_cb(struct ofono_emulator *em,
struct ofono_emulator_request *req, void *userdata)
{
+ struct ofono_gprs *gprs = userdata;
+ struct idmap *cid = gprs-cid_map;
struct ofono_error result;
char buf[256];
@@ -2876,7 +2896,8 @@ static void cgdcont_cb(struct ofono_emulator *em,
switch (ofono_emulator_request_get_type(req)) {
case OFONO_EMULATOR_REQUEST_TYPE_SUPPORT:
/* TODO: check additionnal parameters */
- snprintf(buf, 255, +CGDCONT:
(1-2),\IP\,,,(0-2),(0,1,2,3,4));
+ snprintf(buf, 255, +CGDCONT:
(1-%d),\IP\,,,(0-2),(0,1,2,3,4),
+ idmap_get_size(cid));
, but use the size of the cid_map in the CGDCONT=? implementation below.
These sizes are highly unlikely to match.
However, this brings up a more fundamental question from me. What do
you actually want to expose here? Do you want the emulator to be able
to view / edit contexts exposed over D-Bus?
Or is it enough to simply fake all this and maintain the cgdcont list in
memory for each emulator instance?
ofono_emulator_send_info(em, buf, FALSE);
result.type = OFONO_ERROR_TYPE_NO_ERROR;
ofono_emulator_send_final(em, result);
@@ -2888,7 +2909,8 @@ static void cgdcont_cb(struct ofono_emulator *em,
break;
case OFONO_EMULATOR_REQUEST_TYPE_QUERY:
- result.type = OFONO_ERROR_TYPE_FAILURE;
+ ofono_gprs_list_contexts(em, gprs);
+ result.type = OFONO_ERROR_TYPE_NO_ERROR;
ofono_emulator_send_final(em, result);
break;
case OFONO_EMULATOR_REQUEST_TYPE_SET:
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
