RE: [PATCH 2/2] ifxmodem: add enable/disable ctm support
Hi Marcel,
ofono-boun...@ofono.org wrote:
+static void xctms_modify_cb(gboolean ok, GAtResult *result,
gpointer +user_data) { + struct cb_data *cbd = user_data;
+ofono_ctm_set_cb_t cb = cbd-cb;
+struct ofono_error error;
+const char *setting = NULL;
+struct ofono_ctm *ctm = cbd-user;
+struct ctm_data *ctmd = ofono_ctm_get_data(ctm);
+ofono_bool_t enable = ctmd-enable;
+
+decode_at_error(error, g_at_result_final_response(result)); +
+if (!ok) {
+cb(error, cbd-data);
+return;
+}
+
+if (g_strcmp0(ctmd-audio_setting, FULL_DUPLEX) == 0) +
setting
= 0,0,0,0,0,0,0; + else if (g_strcmp0(ctmd-audio_setting,
BURSTMODE_48KHZ) == 0) + setting = 0,0,8,0,2,0,0; +
else if
(g_strcmp0(ctmd-audio_setting, BURSTMODE_96KHZ) == 0) +
setting
= 0,0,9,0,2,0,0; + + if (setting) {
+char xdrv_buf[64];
+
+/* configure source */
+snprintf(xdrv_buf, sizeof(xdrv_buf),
AT+XDRV=40,4,%d,%d,%s,%s,
+4, +
0,
+setting,
+enable ? 2,5 : 0,0);
+g_at_chat_send(ctmd-chat, xdrv_buf, xdrv_prefix, NULL,
NULL,
+NULL); +
+/* configure destination */
+snprintf(xdrv_buf, sizeof(xdrv_buf),
AT+XDRV=40,5,%d,%d,%s,%s,
+3, +
0,
+setting,
+enable ? 2,6 : 0,0);
+
+g_at_chat_send(ctmd-chat, xdrv_buf, xdrv_prefix, NULL,
NULL,
+NULL); +}
Now this is something I don't like at all. It is copied code from the
modem plugin.
Its the same audio configuration code except that there is a new
parameter added
at the end of the parameter list for TTY case.
The initial discussion was that we need to configure XDRV
only once during init and never have to touch it again. That
seems to be not true anymore. So what is the deal here?
Audio source/destination parameter includes configuration and transducer
mode
TRANSDUCER is the difference. Incase of voice call it is set to
default(0) whereas
for TTY call it is set to TRANSDUCER TTY( 5 for source and 6 for
destination).
TTY call - Last 2 parameters are 2,5 and 2,6 for the source(uplink) and
destination(downlink) respectively.
Voice call - Last 2 parameters are 0,0.
Also if this is required, we might need to figure out a
complete different way of handling this. We can't have this
in two places since that means a full disconnect. Maybe
putting this into the audio settings atom might be better.
However before we can do anything, I have to understand the
semantics behind XDRV, normal voice calls and TTY calls.
Correct me if I'm wrong. If we move this to the audio settings atom,
then I'm
afraid that it will end up in used by only ifx modem.
We wanna be consistent, so pleace do ifx_ctm_init(void) here.
+ofono_ctm_driver_register(driver);
+}
+
+void ifx_ctm_exit()
+{
Same here. And for extra bonus points, I accept patches that
fixes this inside the whole oFono tree ;)
It is coding style rule M15 now.
Separate set of patches sent for the M15 coding style rule fix for the
whole oFono tree.
Regards,
Jeevaka
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
RE: [PATCH 2/2] ifxmodem: add enable/disable ctm support
Hi Jeevaka, Now this is something I don't like at all. It is copied code from the modem plugin. Its the same audio configuration code except that there is a new parameter added at the end of the parameter list for TTY case. I really hate duplicating magic numbers in two places. Can we do this without +XDRV for now and put a /* TODO mark */ in the code. I do need to think about this audio settings handling a bit more. So you might have to keep the +XDRV local in your code for testing, but I'd rather get the other TTY logic in place and worry about the audio stuff in a second round of patches. Also if this is required, we might need to figure out a complete different way of handling this. We can't have this in two places since that means a full disconnect. Maybe putting this into the audio settings atom might be better. However before we can do anything, I have to understand the semantics behind XDRV, normal voice calls and TTY calls. Correct me if I'm wrong. If we move this to the audio settings atom, then I'm afraid that it will end up in used by only ifx modem. I am not following. This whole stuff is IFX specific. So yes, it will only be used by IFX. All other vendors have to do their own stuff. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] ifxmodem: add enable/disable ctm support
Hi Jeevaka,
Makefile.am |3 +-
drivers/ifxmodem/ctm.c | 243
+++
drivers/ifxmodem/ifxmodem.c |2 +
drivers/ifxmodem/ifxmodem.h |3 +
4 files changed, 250 insertions(+), 1 deletions(-)
create mode 100644 drivers/ifxmodem/ctm.c
diff --git a/Makefile.am b/Makefile.am
index 8ad01cd..e6494b1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -222,7 +222,8 @@ builtin_sources += drivers/atmodem/atutil.h \
drivers/ifxmodem/audio-settings.c \
drivers/ifxmodem/radio-settings.c \
drivers/ifxmodem/gprs-context.c \
- drivers/ifxmodem/stk.c
+ drivers/ifxmodem/stk.c \
+ drivers/ifxmodem/ctm.c
builtin_modules += stemodem
builtin_sources += drivers/atmodem/atutil.h \
diff --git a/drivers/ifxmodem/ctm.c b/drivers/ifxmodem/ctm.c
new file mode 100644
index 000..17c7f5c
--- /dev/null
+++ b/drivers/ifxmodem/ctm.c
@@ -0,0 +1,243 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2008-2010 Intel Corporation. All rights reserved.
+ *
+ * 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
+
+#define _GNU_SOURCE
+#include string.h
+#include stdlib.h
+#include stdio.h
+#include errno.h
+
+#include glib.h
+
+#include ofono/log.h
+#include ofono/modem.h
+#include ofono/ctm.h
+
+#include gatchat.h
+#include gatresult.h
+
+#include ifxmodem.h
+
+static const char *none_prefix[] = { NULL };
+static const char *xctms_prefix[] = { XCTMS:, NULL };
+static const char *xdrv_prefix[] = { XDRV:, NULL };
+
+struct ctm_data {
+ GAtChat *chat;
+ const char * audio_setting;
you have an extra space between * and audio_setting. Please remove that.
+ ofono_bool_t enable;
+};
+
+static void xctms_query_cb(gboolean ok, GAtResult *result, gpointer
user_data)
+{
+ struct cb_data *cbd = user_data;
+ ofono_ctm_query_cb_t cb = cbd-cb;
+ struct ofono_error error;
+ GAtResultIter iter;
+ int value;
+ ofono_bool_t enable;
+
+ decode_at_error(error, g_at_result_final_response(result));
+
+ if (!ok) {
+ cb(error, -1, cbd-data);
+ return;
+ }
+
+ g_at_result_iter_init(iter, result);
+
+ if (g_at_result_iter_next(iter, XCTMS:) == FALSE)
+ goto error;
+
+ if (g_at_result_iter_next_number(iter, value) == FALSE)
+ goto error;
+
+ /* FULL TTY mode status only sent to oFono */
+ enable = (value == 1) ? TRUE : FALSE;
+
+ cb(error, enable, cbd-data);
+
+ return;
+
+error:
+ CALLBACK_WITH_FAILURE(cb, -1, cbd-data);
+}
+
+static void ifx_query_tty(struct ofono_ctm *ctm, ofono_ctm_query_cb_t cb,
+ void *data)
+{
+ struct ctm_data *ctmd = ofono_ctm_get_data(ctm);
+ struct cb_data *cbd = cb_data_new(cb, data);
+
+ if (cbd == NULL)
+ goto error;
+
+ if (g_at_chat_send(ctmd-chat, AT+XCTMS?, xctms_prefix,
+ xctms_query_cb, cbd, g_free) 0)
+ return;
+
+error:
+ g_free(cbd);
+
+ CALLBACK_WITH_FAILURE(cb, -1, data);
+}
+
+static void xctms_modify_cb(gboolean ok, GAtResult *result, gpointer
user_data)
+{
+ struct cb_data *cbd = user_data;
+ ofono_ctm_set_cb_t cb = cbd-cb;
+ struct ofono_error error;
+ const char *setting = NULL;
+ struct ofono_ctm *ctm = cbd-user;
+ struct ctm_data *ctmd = ofono_ctm_get_data(ctm);
+ ofono_bool_t enable = ctmd-enable;
+
+ decode_at_error(error, g_at_result_final_response(result));
+
+ if (!ok) {
+ cb(error, cbd-data);
+ return;
+ }
+
+ if (g_strcmp0(ctmd-audio_setting, FULL_DUPLEX) == 0)
+ setting = 0,0,0,0,0,0,0;
+ else if (g_strcmp0(ctmd-audio_setting, BURSTMODE_48KHZ) == 0)
+ setting = 0,0,8,0,2,0,0;
+ else if (g_strcmp0(ctmd-audio_setting, BURSTMODE_96KHZ) == 0)
+ setting = 0,0,9,0,2,0,0;
+
+ if (setting) {
+ char xdrv_buf[64];
+
+ /* configure source */
+ snprintf(xdrv_buf, sizeof(xdrv_buf),
