sim800: adding support for SimCom SIM800 modem
PATCH 1/3 sim800: adding support for SimCom SIM800 modem
From: clem <[email protected]>
--- Makefile.am | 4 + plugins/sim800.c | 463 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 467 insertions(+) create mode 100644 plugins/sim800.c
Makefile.am +4 -0
@@ -496,6 +496,10 @@ builtin_sources += plugins/speedupcdma.c builtin_modules += samsung builtin_sources += plugins/samsung.c +builtin_modules += sim800 +builtin_sources += plugins/sim800.c + + builtin_modules += sim900 builtin_sources += plugins/sim900.c
plugins/sim800.c new file +463 -0
463 lines not on this page · raw message
Signature · 1 line
2.7.4
PATCH 2/3 sim800: adding udev detection
From: clem <[email protected]>
--- plugins/udevng.c | 1 + 1 file changed, 1 insertion(+)
plugins/udevng.c +1 -0
@@ -1300,6 +1300,7 @@ static struct { { "cinterion", setup_serial_modem }, { "nokiacdma", setup_serial_modem }, { "sim900", setup_serial_modem }, + { "sim800", setup_serial_modem }, { "wavecom", setup_wavecom }, { "tc65", setup_tc65 }, { "ehs6", setup_ehs6 },
Signature · 1 line
2.7.4
PATCH 3/3 sim800: adding documentation and updating AUTHORS
In reply to Clement VielFrom: clem <[email protected]>
--- AUTHORS | 1 + doc/sim800-modem.txt | 7 +++++++ 2 files changed, 8 insertions(+) create mode 100644 doc/sim800-modem.txt
AUTHORS +1 -0
@@ -137,3 +137,4 @@ Varun Gargi <[email protected]> Florent Beillonnet <[email protected]> Martin Hundebøll <[email protected]> Julien Tournier <[email protected]> +Clement Viel <[email protected]>
doc/sim800-modem.txt new file +7 -0
@@ -0,0 +1,7 @@ +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=="gsmtty3", ENV{OFONO_DRIVER}="sim800"
Signature · 1 line
2.7.4
Hi Clement,
Quoted 1 line · Clement Viel
From: clem <[email protected]>
Your author information is still broken for the actual commits. Please fix that.
Quoted 12 lines
--- Makefile.am | 4 + plugins/sim800.c | 463 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 467 insertions(+)create mode 100644
plugins/sim800.cMakefile.am +0
-0
Makefile.am builtin_sources += plugins/speedupcdma.c · line 502
Quoted 7 lines
Makefile.am +4
-0@@ -496,6 +496,10 @@ builtin_sources += plugins/speedupcdma.c builtin_modules += samsung builtin_sources += plugins/samsung.c +builtin_modules += sim800 +builtin_sources += plugins/sim800.c + +
One empty line please
Quoted 9 lines
Makefile.am +0
-0builtin_modules += sim900 builtin_sources += plugins/sim900.cplugins/sim800.c new file +0
-0
plugins/sim800.c line 5
Quoted 5 lines
plugins/sim800.c new file +5
-0@@ -0,0 +1,463 @@ +/* +* +* oFono - Open Source Telephony +* +* Copyright (C) 2008-2011 Intel Corporation. All rights reserved.
Not updating the copyright?
Quoted 54 lines
plugins/sim800.c new file +54
-0+* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +* +*/ + +#ifdef HAVE_CONFIG_H +#include <config.h> +#endif + +#include <errno.h> +#include <stdlib.h> +#include <unistd.h> +#include <glib.h> +#include <gatchat.h> +#include <gattty.h> +#include <gatmux.h> + +#define OFONO_API_SUBJECT_TO_CHANGE +#include <ofono/plugin.h> +#include <ofono/modem.h> +#include <ofono/devinfo.h> +#include <ofono/netreg.h> +#include <ofono/sim.h> +#include <ofono/sms.h> +#include <ofono/ussd.h> +#include <ofono/gprs.h> +#include <ofono/gprs-context.h> +#include <ofono/phonebook.h> +#include <ofono/history.h> +#include <ofono/log.h> +#include <ofono/voicecall.h> +#include <ofono/call-volume.h> +#include <drivers/atmodem/vendor.h> + +#define NUM_DLC 5 + +#define VOICE_DLC 0 +#define NETREG_DLC 1 +#define SMS_DLC 2 +#define GPRS_DLC 3 +#define SETUP_DLC 4 + +static char *dlc_prefixes[NUM_DLC ] = {"Voice: " ,"Net: " ,"SMS: " ,
plugins/sim800.c line 69
Quoted 10 lines
plugins/sim800.c new file +10
-0+ "GPRS: " , "Setup: "}; + +static const char *none_prefix[] = {NULL }; + +struct sim800_data { +GIOChannel *device; +GAtMux *mux; +GAtChat * dlcs[NUM_DLC ]; +guint frame_size; +int mux_not_supported;
bool ?
plugins/sim800.c line 72
Quoted 3 lines
plugins/sim800.c new file +3
-0+}; + +
No double empty lines please
plugins/sim800.c line 85
Quoted 13 lines
plugins/sim800.c new file +13
-0+static void mux_ready_notify (GAtResult *result,gpointer user_data) +{ +struct ofono_modem *modem = user_data; +struct sim800_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 ]); + +
Wrong indentation & coding style...
Quoted 14 lines
plugins/sim800.c new file +14
-0+ 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); + } + notified =1 ; + +} + +
<snip>
Quoted 4 lines
plugins/sim800.c new file +4
-0+static void mux_setup_cb (gboolean ok,GAtResult *result,gpointer user_data) +{ +struct ofono_modem *modem = user_data; +struct sim800_data *data =ofono_modem_get_data (modem);
plugins/sim800.c line 299
Quoted 10 lines
plugins/sim800.c new file +10
-0+ +DBG ("" ); + +g_at_chat_unref (data->dlcs[SETUP_DLC ]); + data->dlcs[SETUP_DLC ] =NULL ; + +if (!ok) + goto error; + + data->mux_not_supported =0 ;
What does this variable do?
plugins/sim800.c line 312
Quoted 13 lines
plugins/sim800.c new file +13
-0+setup_internal_mux (modem); +return ; + +error: +DBG ("If you are on a SIM800H modem with firmware version is \ +1308B02SIM800H32_BT or lower, CMUX command is not supported thus \ +preventing GPRS, Voice and SMS managers \ +to be executed simultaneously." ); +/* +* If your use of SIM800 does not need simultaneous use of Voice, SMS +* and GPRS, you can ignore the error, set mux_not_supported=1 and comment +* setup_internal_mux function, refactor this code to use only one DLC. +* Else, you must contact SIMCOM to get a firmware update.
I would drop all these hacks and document the required firmware version in doc/sim800-modem.txt.
Quoted 10 lines
plugins/sim800.c new file +10
-0+*/ +shutdown_device (data); +ofono_modem_set_powered (modem, FALSE); +} + +static void cfun_enable (gboolean ok,GAtResult *result,gpointer user_data) +{ +struct ofono_modem *modem = user_data; +struct sim800_data *data =ofono_modem_get_data (modem); +
plugins/sim800.c line 332
Quoted 10 lines
plugins/sim800.c new file +10
-0+DBG ("" ); + +if (!ok) { +g_at_chat_unref (data->dlcs[SETUP_DLC ]); + data->dlcs[SETUP_DLC ] =NULL ; +ofono_modem_set_powered (modem, FALSE); +return ; + } + +g_at_chat_send (data->dlcs[SETUP_DLC ],"AT+CMUX=0,0,5,128,10,3,30,10,2" ,NULL ,mux_setup_cb, modem,NULL );
Not our coding style.
plugins/sim800.c line 96
Quoted 3 lines
plugins/sim800.c new file +3
-0+ +} +
<snip>
I ran a quick diff between this and sim900 driver and there's really not enough differences to warrant a completely separate plugin. Can't we just simply query the model version and act accordingly?
Regards,
-Denis
Hi Denis,
Quoted 215 lines · Denis Kenzior
Hi Clement,
Quoted 1 line · Clement Viel
From: clem <[email protected]>
Your author information is still broken for the actual commits. Please fix that.
Quoted 12 lines
--- Makefile.am | 4 + plugins/sim800.c | 463 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 467 insertions(+) create mode 100644 plugins/sim800.cMakefile.am +0
-0Makefile.am builtin_sources += plugins/speedupcdma.c · line 501
Quoted 6 lines
Makefile.am +4
-0@@ -496,6 +496,10 @@ builtin_sources += plugins/speedupcdma.c builtin_modules += samsung builtin_sources += plugins/samsung.c +builtin_modules += sim800 +builtin_sources += plugins/sim800.c + +One empty line please
plugins/sim800.c line 5
Quoted 13 lines
Makefile.am +0
-0builtin_modules += sim900 builtin_sources += plugins/sim900.cplugins/sim800.c new file +5
-0@@ -0,0 +1,463 @@ +/* +* +* oFono - Open Source Telephony +* +* Copyright (C) 2008-2011 Intel Corporation. All rights reserved. Not updating the copyright?
Quoted 54 lines
plugins/sim800.c new file +54
-0+* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +* +*/ + +#ifdef HAVE_CONFIG_H +#include <config.h> +#endif + +#include <errno.h> +#include <stdlib.h> +#include <unistd.h> +#include <glib.h> +#include <gatchat.h> +#include <gattty.h> +#include <gatmux.h> + +#define OFONO_API_SUBJECT_TO_CHANGE +#include <ofono/plugin.h> +#include <ofono/modem.h> +#include <ofono/devinfo.h> +#include <ofono/netreg.h> +#include <ofono/sim.h> +#include <ofono/sms.h> +#include <ofono/ussd.h> +#include <ofono/gprs.h> +#include <ofono/gprs-context.h> +#include <ofono/phonebook.h> +#include <ofono/history.h> +#include <ofono/log.h> +#include <ofono/voicecall.h> +#include <ofono/call-volume.h> +#include <drivers/atmodem/vendor.h> + +#define NUM_DLC 5 + +#define VOICE_DLC 0 +#define NETREG_DLC 1 +#define SMS_DLC 2 +#define GPRS_DLC 3 +#define SETUP_DLC 4 + +static char *dlc_prefixes[NUM_DLC ] = {"Voice: " ,"Net: " ,"SMS: " ,plugins/sim800.c line 69
Quoted 10 lines
plugins/sim800.c new file +10
-0+ "GPRS: " , "Setup: "}; + +static const char *none_prefix[] = {NULL }; + +struct sim800_data { +GIOChannel *device; +GAtMux *mux; +GAtChat * dlcs[NUM_DLC ]; +guint frame_size; +int mux_not_supported;bool ?
plugins/sim800.c line 72
Quoted 3 lines
plugins/sim800.c new file +3
-0+}; + +No double empty lines please
plugins/sim800.c line 85
Quoted 13 lines
plugins/sim800.c new file +13
-0+static void mux_ready_notify (GAtResult *result,gpointer user_data) +{ +struct ofono_modem *modem = user_data; +struct sim800_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 ]); + +Wrong indentation & coding style...
Quoted 14 lines
plugins/sim800.c new file +14
-0+ 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); + } + notified =1 ; + +} + +<snip>
Quoted 4 lines
plugins/sim800.c new file +4
-0+static void mux_setup_cb (gboolean ok,GAtResult *result,gpointer user_data) +{ +struct ofono_modem *modem = user_data; +struct sim800_data *data =ofono_modem_get_data (modem);plugins/sim800.c line 299
Quoted 10 lines
plugins/sim800.c new file +10
-0+ +DBG ("" ); + +g_at_chat_unref (data->dlcs[SETUP_DLC ]); + data->dlcs[SETUP_DLC ] =NULL ; + +if (!ok) + goto error; + + data->mux_not_supported =0 ;What does this variable do?
plugins/sim800.c line 312
Quoted 13 lines
plugins/sim800.c new file +13
-0+setup_internal_mux (modem); +return ; + +error: +DBG ("If you are on a SIM800H modem with firmware version is \ +1308B02SIM800H32_BT or lower, CMUX command is not supported thus \ +preventing GPRS, Voice and SMS managers \ +to be executed simultaneously." ); +/* +* If your use of SIM800 does not need simultaneous use of Voice, SMS +* and GPRS, you can ignore the error, set mux_not_supported=1 and comment +* setup_internal_mux function, refactor this code to use only one DLC. +* Else, you must contact SIMCOM to get a firmware update. I would drop all these hacks and document the required firmware version in
doc/sim800-modem.txt.Quoted 10 lines
plugins/sim800.c new file +10
-0+*/ +shutdown_device (data); +ofono_modem_set_powered (modem, FALSE); +} + +static void cfun_enable (gboolean ok,GAtResult *result,gpointer user_data) +{ +struct ofono_modem *modem = user_data; +struct sim800_data *data =ofono_modem_get_data (modem); +plugins/sim800.c line 332
Quoted 10 lines
plugins/sim800.c new file +10
-0+DBG ("" ); + +if (!ok) { +g_at_chat_unref (data->dlcs[SETUP_DLC ]); + data->dlcs[SETUP_DLC ] =NULL ; +ofono_modem_set_powered (modem, FALSE); +return ; + } + +g_at_chat_send (data->dlcs[SETUP_DLC ],"AT+CMUX=0,0,5,128,10,3,30,10,2" ,NULL ,mux_setup_cb, modem,NULL );Not our coding style.
Quoted 3 lines
plugins/sim800.c new file +3
-0+ +} +<snip>
Quoted 3 lines
I ran a quick diff between this and sim900 driver and there's really not enough differences to warrant a completely separate plugin. Can't we just simply query the model version and act accordingly?
My opinion is that the sim800 driver is much to be needed as Simcom considers the sim800 as the new sim900. I "fear" that merging the two drivers will end up in a big file having a lot of "if...else" for every different feature between those two drivers. Whereas having the two separate for some time will allow to address the use of sim800's new features and support legacy sim900.
But as you are the maintainer, I think the decision is yours. Tell me and i'll modify my patches accordingly then preventing too much noisy commits :)
Quoted 3 lines
Regards,
-Denis
Regards
Clement
Hi Clement,
Quoted 9 lines
Quoted 4 lines
I ran a quick diff between this and sim900 driver and there's really not enough differences to warrant a completely separate plugin. Can't we just simply query the model version and act accordingly?
My opinion is that the sim800 driver is much to be needed as Simcom considers the sim800 as the new sim900. I "fear" that merging the two drivers will end up in a big file having a lot of "if...else" for every different feature between those two drivers. Whereas having the two separate for some time will allow to address the use of sim800's new features and support legacy sim900.
It might not be as bad as you think. Besides, if we determine at some future date, that sim800 and sim900 are different enough to warrant different drivers, then we have the option of splitting them out into separate drivers at that time.
Quoted 3 lines
But as you are the maintainer, I think the decision is yours. Tell me and i'll modify my patches accordingly then preventing too much noisy commits :)
My feeling is that we should merge these. The only difference I see right now is a VENDOR flag tweak and an SMS Ready notification handling.
These don't amount to much and could be easily handled by the existing
driver.
Regards,
-Denis