* [PATCH 1/3] sim800: add support for sim800 modem.
@ 2018-10-15 17:27 Clement Viel
2018-10-15 17:27 ` [PATCH 2/3] sim800: add documentation and modify AUTHORS file Clement Viel
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Clement Viel @ 2018-10-15 17:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4267 bytes --]
---
plugins/sim900.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 80 insertions(+), 9 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..9f3f334 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -24,6 +24,7 @@
#endif
#include <errno.h>
+#include <string.h>
#include <stdlib.h>
#include <glib.h>
#include <gatchat.h>
@@ -60,13 +61,59 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
static const char *none_prefix[] = { NULL };
+typedef enum type {
+ SIM800,
+ SIM900,
+ SIM_UNKNOWN,
+} type_t;
+
struct sim900_data {
GIOChannel *device;
GAtMux *mux;
GAtChat * dlcs[NUM_DLC];
guint frame_size;
+ type_t modem_type;
};
+static void set_modem_type (struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ const char *path = ofono_modem_get_path(modem);
+
+ if (strstr(path, "sim800"))
+ data->modem_type = SIM800;
+ else if (strstr(path, "sim900"))
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIM_UNKNOWN;
+
+}
+
+static void mux_ready_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs = NULL;
+ struct ofono_gprs_context *gc;
+ static int notified = 0;
+
+ if (!notified) {
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[SMS_DLC]);
+
+
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
+
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[GPRS_DLC]);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }
+
+}
+
static int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -79,6 +126,7 @@ static int sim900_probe(struct ofono_modem *modem)
ofono_modem_set_data(modem, data);
+ set_modem_type(modem);
return 0;
}
@@ -111,6 +159,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;
device = ofono_modem_get_string(modem, key);
+
if (device == NULL)
return NULL;
@@ -232,6 +281,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
goto error;
}
}
+ if (data->modem_type == SIM800) {
+ for (i = 0; i<NUM_DLC; i++) {
+ g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
+ }
+ }
ofono_modem_set_powered(modem, TRUE);
@@ -353,18 +407,20 @@ static void sim900_post_sim(struct ofono_modem *modem)
DBG("%p", modem);
- ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
- ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ if (data->modem_type != SIM800) {
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
data->dlcs[SMS_DLC]);
- gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
- if (gprs == NULL)
- return;
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
- gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
"atmodem", data->dlcs[GPRS_DLC]);
- if (gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }
}
static void sim900_post_online(struct ofono_modem *modem)
@@ -391,9 +447,24 @@ static struct ofono_modem_driver sim900_driver = {
.post_online = sim900_post_online,
};
+
+static struct ofono_modem_driver sim800_driver = {
+ .name = "sim800",
+ .probe = sim900_probe,
+ .remove = sim900_remove,
+ .enable = sim900_enable,
+ .disable = sim900_disable,
+ .pre_sim = sim900_pre_sim,
+ .post_sim = sim900_post_sim,
+ .post_online = sim900_post_online,
+};
+
static int sim900_init(void)
{
- return ofono_modem_driver_register(&sim900_driver);
+ int ret = 0;
+ ret = ofono_modem_driver_register(&sim800_driver);
+ ret += ofono_modem_driver_register(&sim900_driver);
+ return ret;
}
static void sim900_exit(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/3] sim800: add documentation and modify AUTHORS file
2018-10-15 17:27 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
@ 2018-10-15 17:27 ` Clement Viel
2018-11-01 14:48 ` Denis Kenzior
2018-10-15 17:27 ` [PATCH 3/3] sim800: add udev detection Clement Viel
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Clement Viel @ 2018-10-15 17:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1131 bytes --]
---
AUTHORS | 1 +
doc/sim800-modem.txt | 11 +++++++++++
2 files changed, 12 insertions(+)
create mode 100644 doc/sim800-modem.txt
diff --git a/AUTHORS b/AUTHORS
index 2d360e6..a8362c8 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -138,3 +138,4 @@ Florent Beillonnet <florent.beillonnet@gmail.com>
Martin Hundebøll <martin@geanix.com>
Julien Tournier <tournier.julien@gmail.com>
Nandini Rebello <nandini.rebello@intel.com>
+Clement Viel <vielclement@gmail.com>
diff --git a/doc/sim800-modem.txt b/doc/sim800-modem.txt
new file mode 100644
index 0000000..7220cb8
--- /dev/null
+++ b/doc/sim800-modem.txt
@@ -0,0 +1,11 @@
+SIM800 modem usage
+===================
+
+To enable SIM800 module support you need to put the following
+udev rule into appropriate file in /{etc,lib}/udev/rules.d:
+
+KERNEL=="ttyS2", ENV{OFONO_DRIVER}="sim800"
+
+Sim800 driver code is merged with sim900's. So, to add modifications to sim800 driver, sim900.c file must be edited
+accordingly.
+To differentiate the two modems, developpers must use the "modem_type" field of sim900_data structure.
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/3] sim800: add documentation and modify AUTHORS file
2018-10-15 17:27 ` [PATCH 2/3] sim800: add documentation and modify AUTHORS file Clement Viel
@ 2018-11-01 14:48 ` Denis Kenzior
2018-11-02 23:36 ` Clement Viel
0 siblings, 1 reply; 11+ messages in thread
From: Denis Kenzior @ 2018-11-01 14:48 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1858 bytes --]
Hi Clement,
On 10/15/2018 12:27 PM, Clement Viel wrote:
> ---
> AUTHORS | 1 +
Please don't include AUTHORS changes. One of the maintainers will take
care of it.
> doc/sim800-modem.txt | 11 +++++++++++
> 2 files changed, 12 insertions(+)
> create mode 100644 doc/sim800-modem.txt
>
> diff --git a/AUTHORS b/AUTHORS
> index 2d360e6..a8362c8 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -138,3 +138,4 @@ Florent Beillonnet <florent.beillonnet@gmail.com>
> Martin Hundebøll <martin@geanix.com>
> Julien Tournier <tournier.julien@gmail.com>
> Nandini Rebello <nandini.rebello@intel.com>
> +Clement Viel <vielclement@gmail.com>
> diff --git a/doc/sim800-modem.txt b/doc/sim800-modem.txt
> new file mode 100644
> index 0000000..7220cb8
> --- /dev/null
> +++ b/doc/sim800-modem.txt
> @@ -0,0 +1,11 @@
> +SIM800 modem usage
> +===================
> +
> +To enable SIM800 module support you need to put the following
> +udev rule into appropriate file in /{etc,lib}/udev/rules.d:
> +
> +KERNEL=="ttyS2", ENV{OFONO_DRIVER}="sim800"
> +
> +Sim800 driver code is merged with sim900's. So, to add modifications to sim800 driver, sim900.c file must be edited
> +accordingly.
> +To differentiate the two modems, developpers must use the "modem_type" field of sim900_data
typo
structure.
>
Also, we can't have people hand-editing driver code to distinguish
between one device and another. You've already went to the trouble of
creating a separate sim800 driver (which I do not think is the right
approach at all), why not take advantage of that fact with an
appropriate probe() function or something?
Anyway, I still think you should be querying +CGMM or ATI or whatever to
figure out the modem model automatically and not rely on the user to set
this up.
Regards,
-Denis
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/3] sim800: add documentation and modify AUTHORS file
2018-11-01 14:48 ` Denis Kenzior
@ 2018-11-02 23:36 ` Clement Viel
0 siblings, 0 replies; 11+ messages in thread
From: Clement Viel @ 2018-11-02 23:36 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]
Hi Denis,
On Thu, Nov 01, 2018 at 09:48:24AM -0500, Denis Kenzior wrote:
> Hi Clement,
>
> On 10/15/2018 12:27 PM, Clement Viel wrote:
> >---
> > AUTHORS | 1 +
>
> Please don't include AUTHORS changes. One of the maintainers will take care
> of it.
>
> > doc/sim800-modem.txt | 11 +++++++++++
> > 2 files changed, 12 insertions(+)
> > create mode 100644 doc/sim800-modem.txt
> >
> >diff --git a/AUTHORS b/AUTHORS
> >index 2d360e6..a8362c8 100644
> >--- a/AUTHORS
> >+++ b/AUTHORS
> >@@ -138,3 +138,4 @@ Florent Beillonnet <florent.beillonnet@gmail.com>
> > Martin Hundebøll <martin@geanix.com>
> > Julien Tournier <tournier.julien@gmail.com>
> > Nandini Rebello <nandini.rebello@intel.com>
> >+Clement Viel <vielclement@gmail.com>
> >diff --git a/doc/sim800-modem.txt b/doc/sim800-modem.txt
> >new file mode 100644
> >index 0000000..7220cb8
> >--- /dev/null
> >+++ b/doc/sim800-modem.txt
> >@@ -0,0 +1,11 @@
> >+SIM800 modem usage
> >+===================
> >+
> >+To enable SIM800 module support you need to put the following
> >+udev rule into appropriate file in /{etc,lib}/udev/rules.d:
> >+
> >+KERNEL=="ttyS2", ENV{OFONO_DRIVER}="sim800"
> >+
> >+Sim800 driver code is merged with sim900's. So, to add modifications to sim800 driver, sim900.c file must be edited
> >+accordingly.
> >+To differentiate the two modems, developpers must use the "modem_type"
> >field of sim900_data
> typo
>
> structure.
> >
>
> Also, we can't have people hand-editing driver code to distinguish between
> one device and another. You've already went to the trouble of creating a
> separate sim800 driver (which I do not think is the right approach at all),
> why not take advantage of that fact with an appropriate probe() function or
> something?
These indications were targeting developpers to warn them about the fact that sim800 and sim900 drivers are merged.
I'll remove these lines
>
> Anyway, I still think you should be querying +CGMM or ATI or whatever to
> figure out the modem model automatically and not rely on the user to set
> this up.
Yep this will be done.
> Regards,
> -Denis
Regards
Clem
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] sim800: add udev detection
2018-10-15 17:27 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-10-15 17:27 ` [PATCH 2/3] sim800: add documentation and modify AUTHORS file Clement Viel
@ 2018-10-15 17:27 ` Clement Viel
2018-10-31 15:16 ` [PATCH 1/3] sim800: add support for sim800 modem =?unknown-8bit?q?Cl=C3=A9ment?= VIEL
2018-11-01 14:53 ` Denis Kenzior
3 siblings, 0 replies; 11+ messages in thread
From: Clement Viel @ 2018-10-15 17:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
---
plugins/udevng.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 3c39e68..202519e 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1300,6 +1300,7 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] sim800: add support for sim800 modem.
2018-10-15 17:27 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-10-15 17:27 ` [PATCH 2/3] sim800: add documentation and modify AUTHORS file Clement Viel
2018-10-15 17:27 ` [PATCH 3/3] sim800: add udev detection Clement Viel
@ 2018-10-31 15:16 ` =?unknown-8bit?q?Cl=C3=A9ment?= VIEL
2018-11-01 14:53 ` Denis Kenzior
3 siblings, 0 replies; 11+ messages in thread
From: =?unknown-8bit?q?Cl=C3=A9ment?= VIEL @ 2018-10-31 15:16 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5741 bytes --]
Hi all,
I sent these patches 2 weeks ago and did not receive any feedback.
It's not urgent at all, I was just wandering whether they'll be
reviewed so I can move on :-)
Regards
Le lun. 15 oct. 2018 à 19:27, Clement Viel <vielclement@gmail.com> a écrit :
>
> ---
> plugins/sim900.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 80 insertions(+), 9 deletions(-)
>
> diff --git a/plugins/sim900.c b/plugins/sim900.c
> index a7728cd..9f3f334 100644
> --- a/plugins/sim900.c
> +++ b/plugins/sim900.c
> @@ -24,6 +24,7 @@
> #endif
>
> #include <errno.h>
> +#include <string.h>
> #include <stdlib.h>
> #include <glib.h>
> #include <gatchat.h>
> @@ -60,13 +61,59 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
>
> static const char *none_prefix[] = { NULL };
>
> +typedef enum type {
> + SIM800,
> + SIM900,
> + SIM_UNKNOWN,
> +} type_t;
> +
> struct sim900_data {
> GIOChannel *device;
> GAtMux *mux;
> GAtChat * dlcs[NUM_DLC];
> guint frame_size;
> + type_t modem_type;
> };
>
> +static void set_modem_type (struct ofono_modem *modem)
> +{
> + struct sim900_data *data = ofono_modem_get_data(modem);
> + const char *path = ofono_modem_get_path(modem);
> +
> + if (strstr(path, "sim800"))
> + data->modem_type = SIM800;
> + else if (strstr(path, "sim900"))
> + data->modem_type = SIM900;
> + else
> + data->modem_type = SIM_UNKNOWN;
> +
> +}
> +
> +static void mux_ready_notify(GAtResult *result, gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct sim900_data *data = ofono_modem_get_data(modem);
> + struct ofono_gprs *gprs = NULL;
> + struct ofono_gprs_context *gc;
> + static int notified = 0;
> +
> + if (!notified) {
> + ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> + data->dlcs[SMS_DLC]);
> +
> +
> + gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> + if (gprs == NULL)
> + return;
> +
> + gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
> + "atmodem", data->dlcs[GPRS_DLC]);
> + if (gc)
> + ofono_gprs_add_context(gprs, gc);
> + }
> +
> +}
> +
> static int sim900_probe(struct ofono_modem *modem)
> {
> struct sim900_data *data;
> @@ -79,6 +126,7 @@ static int sim900_probe(struct ofono_modem *modem)
>
> ofono_modem_set_data(modem, data);
>
> + set_modem_type(modem);
> return 0;
> }
>
> @@ -111,6 +159,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
> GHashTable *options;
>
> device = ofono_modem_get_string(modem, key);
> +
> if (device == NULL)
> return NULL;
>
> @@ -232,6 +281,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
> goto error;
> }
> }
> + if (data->modem_type == SIM800) {
> + for (i = 0; i<NUM_DLC; i++) {
> + g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
> + }
> + }
>
> ofono_modem_set_powered(modem, TRUE);
>
> @@ -353,18 +407,20 @@ static void sim900_post_sim(struct ofono_modem *modem)
>
> DBG("%p", modem);
>
> - ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> - ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> + if (data->modem_type != SIM800) {
> + ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> + ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> data->dlcs[SMS_DLC]);
>
> - gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> - if (gprs == NULL)
> - return;
> + gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> + if (gprs == NULL)
> + return;
>
> - gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
> + gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
> "atmodem", data->dlcs[GPRS_DLC]);
> - if (gc)
> - ofono_gprs_add_context(gprs, gc);
> + if (gc)
> + ofono_gprs_add_context(gprs, gc);
> + }
> }
>
> static void sim900_post_online(struct ofono_modem *modem)
> @@ -391,9 +447,24 @@ static struct ofono_modem_driver sim900_driver = {
> .post_online = sim900_post_online,
> };
>
> +
> +static struct ofono_modem_driver sim800_driver = {
> + .name = "sim800",
> + .probe = sim900_probe,
> + .remove = sim900_remove,
> + .enable = sim900_enable,
> + .disable = sim900_disable,
> + .pre_sim = sim900_pre_sim,
> + .post_sim = sim900_post_sim,
> + .post_online = sim900_post_online,
> +};
> +
> static int sim900_init(void)
> {
> - return ofono_modem_driver_register(&sim900_driver);
> + int ret = 0;
> + ret = ofono_modem_driver_register(&sim800_driver);
> + ret += ofono_modem_driver_register(&sim900_driver);
> + return ret;
> }
>
> static void sim900_exit(void)
> --
> 2.7.4
>
--
----
Clement Viel
Tel : +33688431961
-----
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] sim800: add support for sim800 modem.
2018-10-15 17:27 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
` (2 preceding siblings ...)
2018-10-31 15:16 ` [PATCH 1/3] sim800: add support for sim800 modem =?unknown-8bit?q?Cl=C3=A9ment?= VIEL
@ 2018-11-01 14:53 ` Denis Kenzior
2018-11-02 23:31 ` Clement Viel
3 siblings, 1 reply; 11+ messages in thread
From: Denis Kenzior @ 2018-11-01 14:53 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5259 bytes --]
Hi Clement,
On 10/15/2018 12:27 PM, Clement Viel wrote:
> ---
> plugins/sim900.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 80 insertions(+), 9 deletions(-)
>
> diff --git a/plugins/sim900.c b/plugins/sim900.c
> index a7728cd..9f3f334 100644
> --- a/plugins/sim900.c
> +++ b/plugins/sim900.c
> @@ -24,6 +24,7 @@
> #endif
>
> #include <errno.h>
> +#include <string.h>
> #include <stdlib.h>
> #include <glib.h>
> #include <gatchat.h>
> @@ -60,13 +61,59 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
>
> static const char *none_prefix[] = { NULL };
>
> +typedef enum type {
> + SIM800,
> + SIM900,
> + SIM_UNKNOWN,
> +} type_t;
> +
> struct sim900_data {
> GIOChannel *device;
> GAtMux *mux;
> GAtChat * dlcs[NUM_DLC];
> guint frame_size;
> + type_t modem_type;
> };
>
> +static void set_modem_type (struct ofono_modem *modem)
> +{
> + struct sim900_data *data = ofono_modem_get_data(modem);
> + const char *path = ofono_modem_get_path(modem);
> +
> + if (strstr(path, "sim800"))
> + data->modem_type = SIM800;
> + else if (strstr(path, "sim900"))
> + data->modem_type = SIM900;
> + else
> + data->modem_type = SIM_UNKNOWN;
> +
You can't really rely on the modem path for this. Either query the
model directly or set the type based on the driver selected.
And no empty lines at the end of the function please
> +}
> +
> +static void mux_ready_notify(GAtResult *result, gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct sim900_data *data = ofono_modem_get_data(modem);
> + struct ofono_gprs *gprs = NULL;
> + struct ofono_gprs_context *gc;
> + static int notified = 0;
> +
> + if (!notified) {
> + ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> + data->dlcs[SMS_DLC]);
> +
> +
> + gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> + if (gprs == NULL)
> + return;
> +
> + gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
> + "atmodem", data->dlcs[GPRS_DLC]);
> + if (gc)
> + ofono_gprs_add_context(gprs, gc);
> + }
> +
The indentation and stule is all wrong here.
> +}
> +
> static int sim900_probe(struct ofono_modem *modem)
> {
> struct sim900_data *data;
> @@ -79,6 +126,7 @@ static int sim900_probe(struct ofono_modem *modem)
>
> ofono_modem_set_data(modem, data);
>
> + set_modem_type(modem);
> return 0;
> }
>
> @@ -111,6 +159,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
> GHashTable *options;
>
> device = ofono_modem_get_string(modem, key);
> +
This change is spurious and should not be part of this commit
> if (device == NULL)
> return NULL;
>
> @@ -232,6 +281,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
> goto error;
> }
> }
> + if (data->modem_type == SIM800) {
> + for (i = 0; i<NUM_DLC; i++) {
> + g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
> + }
> + }
Why do you want this notification on all ports? Isn't one enough?
>
> ofono_modem_set_powered(modem, TRUE);
>
> @@ -353,18 +407,20 @@ static void sim900_post_sim(struct ofono_modem *modem)
>
> DBG("%p", modem);
>
> - ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> - ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> + if (data->modem_type != SIM800) {
> + ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> + ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> data->dlcs[SMS_DLC]);
>
> - gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> - if (gprs == NULL)
> - return;
> + gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> + if (gprs == NULL)
> + return;
>
> - gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
> + gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
> "atmodem", data->dlcs[GPRS_DLC]);
> - if (gc)
> - ofono_gprs_add_context(gprs, gc);
> + if (gc)
> + ofono_gprs_add_context(gprs, gc);
> + }
Som SIM800 doesn't get any of these atoms?
> }
>
> static void sim900_post_online(struct ofono_modem *modem)
> @@ -391,9 +447,24 @@ static struct ofono_modem_driver sim900_driver = {
> .post_online = sim900_post_online,
> };
>
> +
> +static struct ofono_modem_driver sim800_driver = {
> + .name = "sim800",
> + .probe = sim900_probe,
> + .remove = sim900_remove,
> + .enable = sim900_enable,
> + .disable = sim900_disable,
> + .pre_sim = sim900_pre_sim,
> + .post_sim = sim900_post_sim,
> + .post_online = sim900_post_online,
> +};
> +
> static int sim900_init(void)
> {
> - return ofono_modem_driver_register(&sim900_driver);
> + int ret = 0;
> + ret = ofono_modem_driver_register(&sim800_driver);
> + ret += ofono_modem_driver_register(&sim900_driver);
> + return ret;
This doesn't really work this way. The init function should either
register all drivers successfully or fail.
> }
>
> static void sim900_exit(void)
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] sim800: add support for sim800 modem.
2018-11-01 14:53 ` Denis Kenzior
@ 2018-11-02 23:31 ` Clement Viel
0 siblings, 0 replies; 11+ messages in thread
From: Clement Viel @ 2018-11-02 23:31 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5994 bytes --]
Hi Denis,
Thanks for the quick review !
On Thu, Nov 01, 2018 at 09:53:58AM -0500, Denis Kenzior wrote:
> Hi Clement,
>
> On 10/15/2018 12:27 PM, Clement Viel wrote:
> >---
> > plugins/sim900.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 80 insertions(+), 9 deletions(-)
> >
> >diff --git a/plugins/sim900.c b/plugins/sim900.c
> >index a7728cd..9f3f334 100644
> >--- a/plugins/sim900.c
> >+++ b/plugins/sim900.c
> >@@ -24,6 +24,7 @@
> > #endif
> > #include <errno.h>
> >+#include <string.h>
> > #include <stdlib.h>
> > #include <glib.h>
> > #include <gatchat.h>
> >@@ -60,13 +61,59 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
> > static const char *none_prefix[] = { NULL };
> >+typedef enum type {
> >+ SIM800,
> >+ SIM900,
> >+ SIM_UNKNOWN,
> >+} type_t;
> >+
> > struct sim900_data {
> > GIOChannel *device;
> > GAtMux *mux;
> > GAtChat * dlcs[NUM_DLC];
> > guint frame_size;
> >+ type_t modem_type;
> > };
> >+static void set_modem_type (struct ofono_modem *modem)
> >+{
> >+ struct sim900_data *data = ofono_modem_get_data(modem);
> >+ const char *path = ofono_modem_get_path(modem);
> >+
> >+ if (strstr(path, "sim800"))
> >+ data->modem_type = SIM800;
> >+ else if (strstr(path, "sim900"))
> >+ data->modem_type = SIM900;
> >+ else
> >+ data->modem_type = SIM_UNKNOWN;
> >+
>
> You can't really rely on the modem path for this. Either query the model
> directly or set the type based on the driver selected.
>
> And no empty lines at the end of the function please
I'll go with the CGMM way.
>
> >+}
> >+
> >+static void mux_ready_notify(GAtResult *result, gpointer user_data)
> >+{
> >+ struct ofono_modem *modem = user_data;
> >+ struct sim900_data *data = ofono_modem_get_data(modem);
> >+ struct ofono_gprs *gprs = NULL;
> >+ struct ofono_gprs_context *gc;
> >+ static int notified = 0;
> >+
> >+ if (!notified) {
> >+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> >+ data->dlcs[SMS_DLC]);
> >+
> >+
> >+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> >+ if (gprs == NULL)
> >+ return;
> >+
> >+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
> >+ "atmodem", data->dlcs[GPRS_DLC]);
> >+ if (gc)
> >+ ofono_gprs_add_context(gprs, gc);
> >+ }
> >+
>
> The indentation and stule is all wrong here.
>
> >+}
> >+
> > static int sim900_probe(struct ofono_modem *modem)
> > {
> > struct sim900_data *data;
> >@@ -79,6 +126,7 @@ static int sim900_probe(struct ofono_modem *modem)
> > ofono_modem_set_data(modem, data);
> >+ set_modem_type(modem);
> > return 0;
> > }
> >@@ -111,6 +159,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
> > GHashTable *options;
> > device = ofono_modem_get_string(modem, key);
> >+
>
> This change is spurious and should not be part of this commit
>
> > if (device == NULL)
> > return NULL;
> >@@ -232,6 +281,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
> > goto error;
> > }
> > }
> >+ if (data->modem_type == SIM800) {
> >+ for (i = 0; i<NUM_DLC; i++) {
> >+ g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
> >+ }
> >+ }
>
> Why do you want this notification on all ports? Isn't one enough?
These URC's are sent on all channels so I am catching them all to ensure all channels are now ready.
> > ofono_modem_set_powered(modem, TRUE);
> >@@ -353,18 +407,20 @@ static void sim900_post_sim(struct ofono_modem *modem)
> > DBG("%p", modem);
> >- ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> >- ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> >+ if (data->modem_type != SIM800) {
> >+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> >+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> > data->dlcs[SMS_DLC]);
> >- gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> >- if (gprs == NULL)
> >- return;
> >+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> >+ if (gprs == NULL)
> >+ return;
> >- gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
> >+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
> > "atmodem", data->dlcs[GPRS_DLC]);
> >- if (gc)
> >- ofono_gprs_add_context(gprs, gc);
> >+ if (gc)
> >+ ofono_gprs_add_context(gprs, gc);
> >+ }
>
> Som SIM800 doesn't get any of these atoms?
SIM800 get these atoms by antoher way : the function setup_internal_mux registers the callback for SMS_READY URC. This is the callback that will call these atoms.
The Sim800 must respect this order when using these atoms else all the interfaces won't be up even if the AT commands returned OK.
>
> > }
> > static void sim900_post_online(struct ofono_modem *modem)
> >@@ -391,9 +447,24 @@ static struct ofono_modem_driver sim900_driver = {
> > .post_online = sim900_post_online,
> > };
> >+
> >+static struct ofono_modem_driver sim800_driver = {
> >+ .name = "sim800",
> >+ .probe = sim900_probe,
> >+ .remove = sim900_remove,
> >+ .enable = sim900_enable,
> >+ .disable = sim900_disable,
> >+ .pre_sim = sim900_pre_sim,
> >+ .post_sim = sim900_post_sim,
> >+ .post_online = sim900_post_online,
> >+};
> >+
> > static int sim900_init(void)
> > {
> >- return ofono_modem_driver_register(&sim900_driver);
> >+ int ret = 0;
> >+ ret = ofono_modem_driver_register(&sim800_driver);
> >+ ret += ofono_modem_driver_register(&sim900_driver);
> >+ return ret;
>
> This doesn't really work this way. The init function should either register
> all drivers successfully or fail.
>
> > }
> > static void sim900_exit(void)
> >
>
> Regards,
> -Denis
I'll add those remarks on the new patchset ( and i hope this one will be the one)!
Regards
Clem
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] sim800: add support for sim800 modem.
@ 2018-11-07 10:14 Clement Viel
2018-11-07 13:47 ` Jonas Bonn
0 siblings, 1 reply; 11+ messages in thread
From: Clement Viel @ 2018-11-07 10:14 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5154 bytes --]
---
plugins/sim900.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 98 insertions(+), 10 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..c12576d 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -24,6 +24,7 @@
#endif
#include <errno.h>
+#include <string.h>
#include <stdlib.h>
#include <glib.h>
#include <gatchat.h>
@@ -60,13 +61,73 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
static const char *none_prefix[] = { NULL };
+typedef enum type {
+ SIM800,
+ SIM900,
+ SIM_UNKNOWN,
+} type_t;
+
struct sim900_data {
GIOChannel *device;
GAtMux *mux;
GAtChat * dlcs[NUM_DLC];
guint frame_size;
+ type_t modem_type;
};
+static void set_modem_type (const char *type, struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+
+ if (strcmp(type, "sim800") == 0)
+ data->modem_type = SIM800;
+ else if (strcmp(type, "sim900") == 0)
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIM_UNKNOWN;
+ DBG("modem type is %d",data->modem_type);
+}
+
+static void mux_ready_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs = NULL;
+ struct ofono_gprs_context *gc;
+
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[SMS_DLC]);
+
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
+
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[GPRS_DLC]);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+
+}
+
+static void check_model(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ GAtResultIter iter;
+ char const *model;
+
+ g_at_result_iter_init(&iter, result);
+
+ while (g_at_result_iter_next(&iter, NULL)) {
+ if (!g_at_result_iter_next_unquoted_string(&iter, &model))
+ continue;
+
+ if (strstr(model, "SIM800"))
+ set_modem_type("sim800", modem);
+ else if (strstr(model, "SIM900"))
+ set_modem_type("sim900", modem);
+ }
+}
+
static int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -78,7 +139,6 @@ static int sim900_probe(struct ofono_modem *modem)
return -ENOMEM;
ofono_modem_set_data(modem, data);
-
return 0;
}
@@ -111,6 +171,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;
device = ofono_modem_get_string(modem, key);
+
if (device == NULL)
return NULL;
@@ -232,6 +293,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
goto error;
}
}
+ if (data->modem_type == SIM800) {
+ for (i = 0; i<NUM_DLC; i++) {
+ g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
+ }
+ }
ofono_modem_set_powered(modem, TRUE);
@@ -294,6 +360,7 @@ static int sim900_enable(struct ofono_modem *modem)
return -EINVAL;
g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
+ g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CGMM", NULL,check_model, modem, NULL);
/* For obtain correct sms service number */
g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CSCS=\"GSM\"", NULL,
@@ -353,18 +420,20 @@ static void sim900_post_sim(struct ofono_modem *modem)
DBG("%p", modem);
- ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
- ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ if (data->modem_type != SIM800) {
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
data->dlcs[SMS_DLC]);
- gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
- if (gprs == NULL)
- return;
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
- gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
"atmodem", data->dlcs[GPRS_DLC]);
- if (gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }
}
static void sim900_post_online(struct ofono_modem *modem)
@@ -391,9 +460,28 @@ static struct ofono_modem_driver sim900_driver = {
.post_online = sim900_post_online,
};
+
+static struct ofono_modem_driver sim800_driver = {
+ .name = "sim800",
+ .probe = sim900_probe,
+ .remove = sim900_remove,
+ .enable = sim900_enable,
+ .disable = sim900_disable,
+ .pre_sim = sim900_pre_sim,
+ .post_sim = sim900_post_sim,
+ .post_online = sim900_post_online,
+};
+
static int sim900_init(void)
{
- return ofono_modem_driver_register(&sim900_driver);
+ int ret = 0;
+ ret = ofono_modem_driver_register(&sim800_driver);
+ if (!ret)
+ return ret;
+ ret = ofono_modem_driver_register(&sim900_driver);
+ if (!ret)
+ return ret;
+ return ret;
}
static void sim900_exit(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/3] sim800: add support for sim800 modem.
2018-11-07 10:14 Clement Viel
@ 2018-11-07 13:47 ` Jonas Bonn
0 siblings, 0 replies; 11+ messages in thread
From: Jonas Bonn @ 2018-11-07 13:47 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 6376 bytes --]
Hi Clement,
On 07/11/2018 11:14, Clement Viel wrote:
> ---
> plugins/sim900.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 98 insertions(+), 10 deletions(-)
>
> diff --git a/plugins/sim900.c b/plugins/sim900.c
> index a7728cd..c12576d 100644
> --- a/plugins/sim900.c
> +++ b/plugins/sim900.c
> @@ -24,6 +24,7 @@
> #endif
>
> #include <errno.h>
> +#include <string.h>
> #include <stdlib.h>
> #include <glib.h>
> #include <gatchat.h>
> @@ -60,13 +61,73 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
>
> static const char *none_prefix[] = { NULL };
>
> +typedef enum type {
> + SIM800,
> + SIM900,
> + SIM_UNKNOWN,
> +} type_t;
> +
type_t is not a great name... furthermore, I don't think ofono wants
typedef'd enums. Just use: enum simcom_model {...} or similar.
> struct sim900_data {
> GIOChannel *device;
> GAtMux *mux;
> GAtChat * dlcs[NUM_DLC];
> guint frame_size;
> + type_t modem_type;
> };
>
> +static void set_modem_type (const char *type, struct ofono_modem *modem)
> +{
> + struct sim900_data *data = ofono_modem_get_data(modem);
> +
> + if (strcmp(type, "sim800") == 0)
> + data->modem_type = SIM800;
> + else if (strcmp(type, "sim900") == 0)
> + data->modem_type = SIM900;
> + else
> + data->modem_type = SIM_UNKNOWN;
> + DBG("modem type is %d",data->modem_type);
> +}
> +
This function is unnecessary... see below.
> +static void mux_ready_notify(GAtResult *result, gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct sim900_data *data = ofono_modem_get_data(modem);
> + struct ofono_gprs *gprs = NULL;
> + struct ofono_gprs_context *gc;
> +
> + ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> + data->dlcs[SMS_DLC]);
> +
> + gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> + if (gprs == NULL)
> + return;
> +
> + gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
> + "atmodem", data->dlcs[GPRS_DLC]);
> + if (gc)
> + ofono_gprs_add_context(gprs, gc);
> +
> +}
> +
> +static void check_model(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + GAtResultIter iter;
> + char const *model;
> +
> + g_at_result_iter_init(&iter, result);
> +
> + while (g_at_result_iter_next(&iter, NULL)) {
> + if (!g_at_result_iter_next_unquoted_string(&iter, &model))
> + continue;
> +
> + if (strstr(model, "SIM800"))
> + set_modem_type("sim800", modem);
> + else if (strstr(model, "SIM900"))
> + set_modem_type("sim900", modem);
Just set the type directly here... no need to pass an arbitrary string
through a secondary function to accomplish that.
Furthermore, the result is exactly "SIM800" or "SIM900", right? No need
for strstr, use strcmp.
> + }
> +}
> +
> static int sim900_probe(struct ofono_modem *modem)
> {
> struct sim900_data *data;
> @@ -78,7 +139,6 @@ static int sim900_probe(struct ofono_modem *modem)
> return -ENOMEM;
>
> ofono_modem_set_data(modem, data);
> -
> return 0;
> }
>
> @@ -111,6 +171,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
> GHashTable *options;
>
> device = ofono_modem_get_string(modem, key);
> +
> if (device == NULL)
> return NULL;
>
> @@ -232,6 +293,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
> goto error;
> }
> }
> + if (data->modem_type == SIM800) {
> + for (i = 0; i<NUM_DLC; i++) {
> + g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
> + }
> + }
>
> ofono_modem_set_powered(modem, TRUE);
>
> @@ -294,6 +360,7 @@ static int sim900_enable(struct ofono_modem *modem)
> return -EINVAL;
>
> g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
> + g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CGMM", NULL,check_model, modem, NULL);
>
> /* For obtain correct sms service number */
> g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CSCS=\"GSM\"", NULL,
> @@ -353,18 +420,20 @@ static void sim900_post_sim(struct ofono_modem *modem)
>
> DBG("%p", modem);
>
> - ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> - ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> + if (data->modem_type != SIM800) {
How about "== SIM900" because that's what this is for.
> + ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
> + ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
> data->dlcs[SMS_DLC]);
>
> - gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> - if (gprs == NULL)
> - return;
> + gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
> + if (gprs == NULL)
> + return;
>
> - gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
> + gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
> "atmodem", data->dlcs[GPRS_DLC]);
> - if (gc)
> - ofono_gprs_add_context(gprs, gc);
> + if (gc)
> + ofono_gprs_add_context(gprs, gc);
> + }
> }
>
> static void sim900_post_online(struct ofono_modem *modem)
> @@ -391,9 +460,28 @@ static struct ofono_modem_driver sim900_driver = {
> .post_online = sim900_post_online,
> };
>
> +
> +static struct ofono_modem_driver sim800_driver = {
> + .name = "sim800",
> + .probe = sim900_probe,
> + .remove = sim900_remove,
> + .enable = sim900_enable,
> + .disable = sim900_disable,
> + .pre_sim = sim900_pre_sim,
> + .post_sim = sim900_post_sim,
> + .post_online = sim900_post_online,
> +};
This is identical to the sim900_driver.
Your driver is already taking into account the modem model and adjusting
functionality accordingly. You don't need to specify a new driver for
the sim800 where the only thing that changes is the name.
> +
> static int sim900_init(void)
> {
> - return ofono_modem_driver_register(&sim900_driver);
> + int ret = 0;
> + ret = ofono_modem_driver_register(&sim800_driver);
> + if (!ret)
> + return ret;
> + ret = ofono_modem_driver_register(&sim900_driver);
> + if (!ret)
> + return ret;
> + return ret;
> }
>
> static void sim900_exit(void)
>
/Jonas
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] sim800: add support for sim800 modem.
@ 2018-10-03 13:28 Clement Viel
0 siblings, 0 replies; 11+ messages in thread
From: Clement Viel @ 2018-10-03 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4267 bytes --]
---
plugins/sim900.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 80 insertions(+), 9 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..9f3f334 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -24,6 +24,7 @@
#endif
#include <errno.h>
+#include <string.h>
#include <stdlib.h>
#include <glib.h>
#include <gatchat.h>
@@ -60,13 +61,59 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
static const char *none_prefix[] = { NULL };
+typedef enum type {
+ SIM800,
+ SIM900,
+ SIM_UNKNOWN,
+} type_t;
+
struct sim900_data {
GIOChannel *device;
GAtMux *mux;
GAtChat * dlcs[NUM_DLC];
guint frame_size;
+ type_t modem_type;
};
+static void set_modem_type (struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ const char *path = ofono_modem_get_path(modem);
+
+ if (strstr(path, "sim800"))
+ data->modem_type = SIM800;
+ else if (strstr(path, "sim900"))
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIM_UNKNOWN;
+
+}
+
+static void mux_ready_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs = NULL;
+ struct ofono_gprs_context *gc;
+ static int notified = 0;
+
+ if (!notified) {
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[SMS_DLC]);
+
+
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
+
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[GPRS_DLC]);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }
+
+}
+
static int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -79,6 +126,7 @@ static int sim900_probe(struct ofono_modem *modem)
ofono_modem_set_data(modem, data);
+ set_modem_type(modem);
return 0;
}
@@ -111,6 +159,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;
device = ofono_modem_get_string(modem, key);
+
if (device == NULL)
return NULL;
@@ -232,6 +281,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
goto error;
}
}
+ if (data->modem_type == SIM800) {
+ for (i = 0; i<NUM_DLC; i++) {
+ g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
+ }
+ }
ofono_modem_set_powered(modem, TRUE);
@@ -353,18 +407,20 @@ static void sim900_post_sim(struct ofono_modem *modem)
DBG("%p", modem);
- ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
- ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ if (data->modem_type != SIM800) {
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
data->dlcs[SMS_DLC]);
- gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
- if (gprs == NULL)
- return;
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
- gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
"atmodem", data->dlcs[GPRS_DLC]);
- if (gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }
}
static void sim900_post_online(struct ofono_modem *modem)
@@ -391,9 +447,24 @@ static struct ofono_modem_driver sim900_driver = {
.post_online = sim900_post_online,
};
+
+static struct ofono_modem_driver sim800_driver = {
+ .name = "sim800",
+ .probe = sim900_probe,
+ .remove = sim900_remove,
+ .enable = sim900_enable,
+ .disable = sim900_disable,
+ .pre_sim = sim900_pre_sim,
+ .post_sim = sim900_post_sim,
+ .post_online = sim900_post_online,
+};
+
static int sim900_init(void)
{
- return ofono_modem_driver_register(&sim900_driver);
+ int ret = 0;
+ ret = ofono_modem_driver_register(&sim800_driver);
+ ret += ofono_modem_driver_register(&sim900_driver);
+ return ret;
}
static void sim900_exit(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-11-07 13:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-15 17:27 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-10-15 17:27 ` [PATCH 2/3] sim800: add documentation and modify AUTHORS file Clement Viel
2018-11-01 14:48 ` Denis Kenzior
2018-11-02 23:36 ` Clement Viel
2018-10-15 17:27 ` [PATCH 3/3] sim800: add udev detection Clement Viel
2018-10-31 15:16 ` [PATCH 1/3] sim800: add support for sim800 modem =?unknown-8bit?q?Cl=C3=A9ment?= VIEL
2018-11-01 14:53 ` Denis Kenzior
2018-11-02 23:31 ` Clement Viel
-- strict thread matches above, loose matches on Subject: below --
2018-11-07 10:14 Clement Viel
2018-11-07 13:47 ` Jonas Bonn
2018-10-03 13:28 Clement Viel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.