Re: [PATCH v4] gprs: Quectel EC21 does not understand AT+CPSB
Hi Lars, On 8/17/20 2:58 AM, poesc...@lemonage.de wrote: From: Lars Poeschel The Quectel EC21 modem does not understand the AT+CPSB command, and we did not find a suitable replacement in the Quectel_EC25_AT_Commands_Manual_V1.3.pdf AT+CPSB gives an error on this modem, so we just skip it. --- drivers/atmodem/gprs.c | 2 ++ 1 file changed, 2 insertions(+) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v3] gprs: Quectel EC21 does not understand AT+CPSB
Hi Lars, On 8/13/20 8:33 AM, poesc...@lemonage.de wrote: From: Lars Poeschel The Quectel EC21 modem does not understand the AT+CPSB command, so aquire the current packet switched bearer from quectel proprietary QIND:"act" URC. --- drivers/atmodem/gprs.c | 49 ++ 1 file changed, 49 insertions(+) @@ -624,6 +667,12 @@ static void gprs_initialized(gboolean ok, GAtResult *result, gpointer user_data) g_at_chat_send(gd->chat, "AT#PSNT=1", none_prefix, NULL, NULL, NULL); break; + case OFONO_VENDOR_QUECTEL_EC2X: + g_at_chat_register(gd->chat, "+QIND", + quectel_qind_notify, FALSE, gprs, NULL); + /* The "QIND: \"act\", ..." URC is activated in +* network-registration.c */ You're using the QIND in network-registration.c to notify CREG technology. And now you're trying to use this here in place of CPSB. This seems fishy to me. Looking at the Quectel AT commands manual, I'd just guess they do not support this bearer concept at all... + break; default: g_at_chat_register(gd->chat, "+CPSB:", cpsb_notify, FALSE, gprs, NULL); Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2 3/3] gprs: Quectel EC21 does not understand AT+CPSB
Hi Lars, oFono driver interface is based on 27.007. So that means values defined in 27.007 do not need to be 'converted'. You can simply feed them in directly if they follow 27.007. Well, yes. I saw this. But unfortunately at this point what is expected are the 27.007 values from AT+CPSB defined in 7.29 (that EC21 quectel modem does not support) and I get the values from AT+CGREG defined in 7.2. Ok, I gotcha. I see now what you're trying to do. The problem is that +CGREG is not really the same as +CPSB. +CGREG only reports the capability of the cell, not the actual bearer being used. This could be for example due to signal strength being too low, etc. If you look at the gprs driver, we never use +CGREG info this way and many modems have a vendor-specific unsolicited notification if +CPSB isn't supported. Since the Bearer is an optional property, maybe its just easier to omit it. Unless you know +CGREG information is the same as +CPSB on Quectel firmware... Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2 3/3] gprs: Quectel EC21 does not understand AT+CPSB
Hi Lars,
On 8/11/20 6:42 AM, poesc...@lemonage.de wrote:
From: Lars Poeschel
The Quectel EC21 modem does not understand the AT+CPSB command, so
aquire the current packet switched bearer from CGREG URC.
---
drivers/atmodem/gprs.c | 32 ++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c
index b637f733..5583d6fa 100644
--- a/drivers/atmodem/gprs.c
+++ b/drivers/atmodem/gprs.c
@@ -40,6 +40,7 @@
#include "gatresult.h"
#include "atmodem.h"
+#include "common.h"
#include "vendor.h"
#define MAX_CONTEXTS 255
@@ -98,6 +99,29 @@ static void list_contexts_data_unref(gpointer user_data)
g_free(ld);
}
+static int act_to_bearer(int act)
+{
+ switch (act) {
+ case 0:
+ case 1:
+ return PACKET_BEARER_GPRS;
+ case 2:
+ return PACKET_BEARER_UMTS;
+ case 3:
+ return PACKET_BEARER_EGPRS;
+ case 4:
+ return PACKET_BEARER_HSDPA;
+ case 5:
+ return PACKET_BEARER_HSUPA;
+ case 6:
+ return PACKET_BEARER_HSUPA_HSDPA;
+ case 7:
+ return PACKET_BEARER_EPS;
+ default:
+ return PACKET_BEARER_NONE;
+ }
+};
+
oFono driver interface is based on 27.007. So that means values defined in
27.007 do not need to be 'converted'. You can simply feed them in directly if
they follow 27.007.
To me this looks like a no-op. Is there a non-obvious reason why this code is
needed?
static void at_cgatt_cb(gboolean ok, GAtResult *result, gpointer user_data)
{
struct cb_data *cbd = user_data;
@@ -342,11 +366,11 @@ static void at_gprs_list_active_contexts(struct
ofono_gprs *gprs,
static void cgreg_notify(GAtResult *result, gpointer user_data)
{
struct ofono_gprs *gprs = user_data;
- int status;
+ int status, tech;
struct gprs_data *gd = ofono_gprs_get_data(gprs);
if (at_util_parse_reg_unsolicited(result, "+CGREG:", ,
- NULL, NULL, NULL, gd->vendor) == FALSE)
+ NULL, NULL, , gd->vendor) == FALSE)
return;
/*
@@ -372,6 +396,8 @@ static void cgreg_notify(GAtResult *result, gpointer
user_data)
}
ofono_gprs_status_notify(gprs, status);
+ if (gd->vendor == OFONO_VENDOR_QUECTEL_EC2X)
+ ofono_gprs_bearer_notify(gprs, act_to_bearer(tech));
To follow up from above, any reason not to omit act_to_bearer() here?
}
static void cgev_notify(GAtResult *result, gpointer user_data)
@@ -624,6 +650,8 @@ static void gprs_initialized(gboolean ok, GAtResult
*result, gpointer user_data)
g_at_chat_send(gd->chat, "AT#PSNT=1", none_prefix,
NULL, NULL, NULL);
break;
+ case OFONO_VENDOR_QUECTEL_EC2X:
+ break;
default:
g_at_chat_register(gd->chat, "+CPSB:", cpsb_notify,
FALSE, gprs, NULL);
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2 1/3] Add a vendor OFONO_VENDOR_QUECTEL_EC2X
Hi Lars, On 8/11/20 6:42 AM, poesc...@lemonage.de wrote: From: Lars Poeschel The distinction between OFONO_VENDOR_QUECTEL and OFONO_VENDOR_QUECTEL_SERIAL does not suffice for EC21/EC25 in some places, so introduce and use a new vendor: OFONO_VENDOR_QUECTEL_EC2X --- drivers/atmodem/sim.c | 1 + drivers/atmodem/sms.c | 2 ++ drivers/atmodem/vendor.h| 1 + drivers/atmodem/voicecall.c | 3 ++- plugins/quectel.c | 2 +- 5 files changed, 7 insertions(+), 2 deletions(-) Patch 1 & 2 applied with minor style cleanups. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 4/5] quectel: Try to update voltage only, when received "vbatt"
Hi Lars, On 8/4/20 7:38 AM, poesc...@lemonage.de wrote: From: Lars Poeschel As there are some more sources of +QIND: activated, do now only update voltage when we get the +QIND: "vbatt",-1 but not on things like +QIND: "act","LTE" or +QIND: "csq",20,99 --- plugins/quectel.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 2/5] quectel: Set URC port to uart1 on EC21
Hi Lars, On 8/4/20 7:38 AM, poesc...@lemonage.de wrote: From: Lars Poeschel Set the URC port of the Quectel EC21 to uart1 when it is used through it's serial port. This setting is saved to non-volatile storage by the modem automatically. --- plugins/quectel.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 1/5] Add a vendor OFONO_VENDOR_QUECTEL_EC2X
Hi Lars, On 8/4/20 7:38 AM, poesc...@lemonage.de wrote: From: Lars Poeschel The distinction between OFONO_VENDOR_QUECTEL and OFONO_VENDOR_QUECTEL_SERIAL does not suffice for EC21/EC25 in some places, so introduce and use a new vendor: OFONO_VENDOR_QUECTEL_EC2X --- drivers/atmodem/lte.c | 2 +- drivers/atmodem/sim.c | 1 + drivers/atmodem/sms.c | 2 ++ drivers/atmodem/vendor.h| 1 + drivers/atmodem/voicecall.c | 3 ++- plugins/quectel.c | 2 +- 6 files changed, 8 insertions(+), 3 deletions(-) This doesn't apply: denkenz@localhost ~/ofono-master $ git am --3way ~/merge/\[PATCH\ 1_5\]\ Add\ a\ vendor\ OFONO_VENDOR_QUECTEL_EC2X.eml Applying: Add a vendor OFONO_VENDOR_QUECTEL_EC2X error: sha1 information is lacking or useless (drivers/atmodem/lte.c). error: could not build fake ancestor Patch failed at 0001 Add a vendor OFONO_VENDOR_QUECTEL_EC2X The chunk in drivers/atmodem/lte.c doesn't look like upstream code at all. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] Revert "quectel: EC21 needs aux channel to be the first mux channel"
Hi Lars, On 8/4/20 6:56 AM, poesc...@lemonage.de wrote: From: Lars Poeschel This reverts commit 1868dbf2b3e5929a7081b03a8ff76d214fd38624. Development for this was done on EC21 firmware version EC21EFAR06A01M4G_BETA0318. It now turns out, that actual release firmware versions for this modem again need the original mux order with aux channel as the second mux channel. (We know for sure for firmware version EC21EFAR06A03M4G.) We do not know for sure when and for what firmware versions quectel did the switch back on the mux order, but we suspect that the "BETA" firmware is the only one with the reversed mux order. This "BETA" firmware was only given out for development purposes and will not appear "in the wild", so we revert the patch here and hope for the best. --- plugins/quectel.c | 61 +++ 1 file changed, 14 insertions(+), 47 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCHv2 1/3] netmon: added PCI, TAC, SNR value
Hi JongSeok, On 7/30/20 9:20 PM, JongSeok Won wrote: To support cell type LTE, the value of PCI, TAC, SNR is added --- include/netmon.h | 3 +++ src/netmon.c | 21 + 2 files changed, 24 insertions(+) All three applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: AT modem on Droid 4: where is my sms?
Hi Pavel, On 7/31/20 5:07 AM, Pavel Machek wrote: Hi! I have problems with getting CNMA to work, so I'm exploring other possibilities. Apparently ofono should be able to work without that.. but is it working properly? ofonod[5218]: < \r\n+CIEV: 1,3\r\n ofonod[5218]: < \r\n+CIEV: 1,4\r\n ofonod[5218]: < \r\n+CMTI: "ME",2\r\n ofonod[5218]: drivers/atmodem/sms.c:at_cmti_notify() Got a CMTI indication at ME, index: 2 ofonod[5218]: > AT+CMGR=2\r ofonod[5218]: < \r\nOK\r\n That's quite strange, CMGR response should have an intermediate response prefixed by +CMGR: prior to the OK. Your modem is just weird ;) ofonod[5218]: > AT+CMGD=2\r ofonod[5218]: < \r\nOK\r\n ofonod[5218]: < \r\n+CIEV: 1,3\r\n ofonod[5218]: < \r\n+CIEV: 1,4\r\n So modem told us we have new message and where it is, but then ofono ... deleted the message without delivering it...? What the stock AT modem driver does is reads the message and deletes it right away. Somehow your modem says 'sure' to the read request without actually reading the message. Maybe the firmware is just too slow or oFono is too fast, or maybe this modem needs extra special magic. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v3] quectel: EC21 needs aux channel to be the first mux channel
Hi Lars, Unfortunately I must come back to this issue. I got hands on a few new EC21s here and guess what ? The mux order is back to the original one again. This means, the aux channel has to be the second channel. So I did a bit of investigation why and when this happened. But information is rare. The modems I originally worked on and created the patch for have Firmware EC21EFAR06A01M4G_BETA0318. (Reversed mux order) The new ones do have version EC21EFAR06A03M4G. (original mux order) I know that there was a version EC21EFAR02A02M4G that did not support cmux at all. Due to some Quectel Confidential Document in a firmware version "R02A03" some bug was fixed in cmux, so cmux must be in there since then. The EC21EFAR06A01M4G_BETA0318 that I have is dated inbetween EC21EFAR02A02M4G and this "R02A03". The mux order must have changed between EC21EFAR06A01M4G_BETA0318 and EC21EFAR06A03M4G. I suspect (without knowing for sure) due to the beta-nature of my firmware, that this is the only firmware with reversed mux order and that they changed it after that and "R02A03" up until EC21EFAR06A03M4G share the same original mux order. According to Quectel the EC21EFAR06A01M4G_BETA0318 firmware I have here is not "out in the wild". The modems I have here are the only ones with this firmware. So my question is what's best to do now ? I feel the best would be to revert this patch. I am very sorry for this. New modems will work and I suspect old modems "out in the wild" will work also. I don't care about supporting the few "BETA0318" modems I have here. I guess just send a revert patch with the reasons outlined in the commit description and I can take it. Another way would be to leave this patch and implement some firmware switch and use reversed mux order for the "BETA0318" only I have and use normal original mux order for all other cases. That would also be fine. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] rilmodem: set proto type during setting initial attach apn
Hi JongSeok, On 7/20/20 4:16 AM, JongSeok Won wrote: Added the protocol type of initial attach apn depends on protocol type in LTE Atom. --- drivers/rilmodem/lte.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 1/2] netmon: support cell type LTE
Hi JongSeok,
On 7/20/20 3:40 AM, JongSeok Won wrote:
---
include/netmon.h | 3 +++
src/netmon.c | 24 +++-
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/include/netmon.h b/include/netmon.h
index a99d6ca9..53f9d393 100644
--- a/include/netmon.h
+++ b/include/netmon.h
@@ -72,6 +72,9 @@ enum ofono_netmon_info {
OFONO_NETMON_INFO_EARFCN, /* int */
OFONO_NETMON_INFO_EBAND, /* int */
OFONO_NETMON_INFO_CQI, /* int */
+ OFONO_NETMON_INFO_PCI, /* int */
+ OFONO_NETMON_INFO_TAC, /* int */
+ OFONO_NETMON_INFO_SNR, /* int */
OFONO_NETMON_INFO_INVALID,
};
diff --git a/src/netmon.c b/src/netmon.c
index 9eacb3ca..320c8425 100644
--- a/src/netmon.c
+++ b/src/netmon.c
@@ -138,7 +138,7 @@ static void netmon_cell_info_dict_append(DBusMessageIter
*dict,
intval = va_arg(*arglist, int);
CELL_INFO_DICT_APPEND(dict, "TimingAdvance",
- intval, uint8_t, DBUS_TYPE_BYTE);
+ intval, uint32_t, DBUS_TYPE_UINT32);
This breaks the NetworkMonitor API since the signature for that particular
property is documented as a 'byte'. From what I recall this value has a range
of 0..63 ?
break;
case OFONO_NETMON_INFO_PSC:
@@ -213,6 +213,28 @@ static void netmon_cell_info_dict_append(DBusMessageIter
*dict,
intval, uint8_t, DBUS_TYPE_BYTE);
break;
+ case OFONO_NETMON_INFO_PCI:
+ intval = va_arg(*arglist, int);
+
+ CELL_INFO_DICT_APPEND(dict, "PhysicalCellId",
+ intval, uint16_t, DBUS_TYPE_UINT16);
+ break;
+
+ case OFONO_NETMON_INFO_TAC:
+ intval = va_arg(*arglist, int);
+
+ CELL_INFO_DICT_APPEND(dict, "TrackingAreaCode",
+ intval, uint16_t, DBUS_TYPE_UINT16);
+ break;
+
+ case OFONO_NETMON_INFO_SNR:
+ intval = va_arg(*arglist, int);
+
+ ofono_dbus_dict_append(dict, "SingalToNoiseRatio",
+ DBUS_TYPE_INT32, );
+
+ break;
+
case OFONO_NETMON_INFO_INVALID:
break;
}
These should also be documented in doc/networkmonitor-api.txt.
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] rilmodem: fix typo error in netmon.c
Hi JongSeok, On 7/20/20 2:34 AM, JongSeok Won wrote: --- drivers/rilmodem/netmon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2 RESEND] udevng: Add support for Quectel BG96 modem
Hi Sean, On 7/11/20 7:04 AM, Sean Nyekjaer wrote: --- plugins/udevng.c | 2 ++ 1 file changed, 2 insertions(+) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2] udevng: Add support for Quectel BG96 modem
Hi Martin, On 7/10/20 2:55 AM, Martin Hundebøll wrote: Hi Denis, Did this patch (and Sean's original one from november) never reach the mailing list? Hmm, I don't recall seeing this one. Feel free to resend it and I can take it. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 1/2] xmm7xxx-enable-esim-feature-in-xmm
Hi Shweta,
On 7/6/20 2:39 PM, shweta wrote:
From: Shweta Jain
---
plugins/xmm7xxx.c | 445 +-
1 file changed, 444 insertions(+), 1 deletion(-)
So when I tried to apply this patch, I got:
Applying: xmm7xxx-enable-esim-feature-in-xmm
.git/rebase-apply/patch:37: trailing whitespace.
.git/rebase-apply/patch:39: space before tab in indent.
GAtChat *chat;
.git/rebase-apply/patch:40: space before tab in indent.
struct ofono_modem *modem;
.git/rebase-apply/patch:41: trailing whitespace.
.git/rebase-apply/patch:42: space before tab in indent.
char *eid;
warning: squelched 347 whitespace errors
error: 352 lines add whitespace errors.
Patch failed at 0001 xmm7xxx-enable-esim-feature-in-xmm
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Please make sure your editor uses only tabs for indentation, does not contain
trailing whitespace.
oFono uses the Linux Kernel coding style, so please make sure that your
submission follows this whenever possible. We have several 'house' rules or
deviations from the Linux coding style that is documented in
doc/coding-style.txt. An easy way to test compliance is to run
scripts/checkpatch.pl from the Linux kernel against your submission prior to
sending it to the list.
diff --git a/plugins/xmm7xxx.c b/plugins/xmm7xxx.c
index a544798..8744299 100644
--- a/plugins/xmm7xxx.c
+++ b/plugins/xmm7xxx.c
@@ -62,6 +62,7 @@
#define OFONO_COEX_INTERFACE OFONO_SERVICE ".intel.LteCoexistence"
#define OFONO_COEX_AGENT_INTERFACE OFONO_SERVICE ".intel.LteCoexistenceAgent"
+#define OFONO_EUICC_LPA_INTERFACE OFONO_SERVICE ".intel.EuiccLpa"
#define NET_BAND_LTE_INVALID 0
#define NET_BAND_LTE_1 101
@@ -73,6 +74,8 @@
static const char *none_prefix[] = { NULL };
static const char *xsimstate_prefix[] = { "+XSIMSTATE:", NULL };
static const char *xnvmplmn_prefix[] = { "+XNVMPLMN:", NULL };
++static const char *ccho_prefix[] = { "+CCHO:", NULL };
++static const char *cgla_prefix[] = { "+CGLA:", NULL };
This '++' seems wrong. Have you tried applying your patch and compile testing
it prior to submission?
struct bt_coex_info {
int safe_tx_min;
@@ -108,8 +111,429 @@ struct xmm7xxx_data {
ofono_bool_t have_sim;
ofono_bool_t sms_phonebook_added;
unsigned int netreg_watch;
+ ofono_bool_t stk_enable;
+ ofono_bool_t enable_euicc;
};
+ /* eUICC Implementation */
+ #define EUICC_EID_CMD "80e2910006BF3E035C015A00"
+ #define EUICC_ISDR_AID "A005591010890100"
+
+ struct xmm7xxx_euicc {
+ GAtChat *chat;
+ struct ofono_modem *modem;
+
+ char *eid;
+ int channel;
+ char *command;
+ int length;
+ DBusMessage *pending;
+ ofono_bool_t is_registered;
+ };
+
+ static char *euiccCmdBytesToHexString(const unsigned char *DataBytes,
+ int dataSize, int *bufferStringSize)
+ {
+ int offset = 0;
+ char *BufferString = NULL;
+ *bufferStringSize = dataSize * 2;
+
+ if (DataBytes) {
+ BufferString = g_new0(char, *bufferStringSize + 1);
+ while (offset < dataSize) {
+ sprintf([offset * 2], "%02x",
+ DataBytes[offset]);
+ offset++;
+ }
+ }
+
+ return BufferString;
+ }
We already have something similar in src/util.[ch]: encode_hex_own_buf(). So
I'd just use that instead.
+
+ static unsigned char *euiccRspStringToBytes(const char *DataString,
+ int dataSize, int *bufferBytesSize)
+ {
+ int offset = 0, tmp;
+ unsigned char *BufferBytes = NULL;
+ *bufferBytesSize = dataSize / 2;
+
+ if (DataString) {
+ BufferBytes = g_new0(unsigned char, *bufferBytesSize);
+
+ while (offset < *bufferBytesSize) {
+ sscanf([offset * 2], "%02x", );
+ BufferBytes[offset] = tmp;
+ offset ++;
+ }
+ }
+
+ return BufferBytes;
+ }
Similarly here, looks like this can use decode_hex_own_buf.
Or better yet, just use ell's utilities for this:
l_util_hexstring
l_util_from_hexstring
+
+ static void euicc_cleanup(void *data)
+ {
+ struct xmm7xxx_euicc *euicc = data;
+
+ g_free(euicc->command);
+ g_free(euicc->eid);
+ g_free(euicc->pending);
This looks wrong. The pending was allocated using dbus_* APIs and should be
deallocated accordingly.
+ g_free(euicc);
+ }
+
+ static void euicc_release_isdr(struct xmm7xxx_euicc *euicc)
+ {
+ DBusMessage *reply;
+ DBusConnection *conn =
Re: [PATCH 2/2] esim-increasing-result-buffer-for-esim-response
Hi Shweta, On 7/6/20 2:39 PM, shweta wrote: From: Shweta Jain --- gatchat/gatresult.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Applied after re-wording the commit message slightly. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 0/2] gemalto: enable ELS81x modem
Hi Sergey, On 6/27/20 6:08 AM, Sergey Matyukevich wrote: Hi all, These two simple patches enable oFono support for ELS81x modem using cdc_ether/cdc_acm drivers. As far as I know, new firmwares for ELS81x enable support for MBIM as well. But I have not yet tried that. Regards, Sergey Sergey Matyukevich (2): plugins: udevng: detect Centirion ELS81x modem plugins: gemalto: enable LTE for ELS81x plugins/gemalto.c | 5 - plugins/udevng.c | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) All applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] huawei: fix AT^SYSCFGEX acqorder "0201"
Hi Jimmy,
On 6/26/20 1:36 AM, Jimmy Gysens wrote:
Commit 6c574ee24a57d0397e4e3c617016bf026405960a ("huawei: the AT^SYSCFGEX
command supports additional modes") has a mistake for acqorder "0201". It
should be UMTS and GSM preferred.
---
drivers/huaweimodem/radio-settings.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Applied, thanks.
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] gprs: clean context properly
Hi Jimmy, On 6/25/20 4:30 AM, Jimmy Gysens wrote: After a context is detached, the context is not properly cleared. In addition to releasing the context: - Reset the context settings (IP, DNS, interface, ...). - Signal the Active flag as false. --- src/gprs.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 1/2] modem: add a driver reset function
Hi Jimmy, On 6/25/20 4:28 AM, Jimmy Gysens wrote: Add a reset function to the modem driver. This function can be used to hard reset the device. This is an alternative in case the (*enable) and (*disable) functions are not supported. I'm confused, how are you even powering the modem up if enable is not implemented? And how would one detect that a reset is needed? Perhaps this makes more sense as a vendor specific API inside plugins/huawei.c ? Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 1/2] huawei: add RejectInfo signal to org.ofono.ConnectionManager
Hi Jimmy,
On 6/25/20 4:28 AM, Jimmy Gysens wrote:
Huawei devices can have support for ^REJINFO unsolicited event. This event
provides useful info, regarding to the network attached state, for higher
level applications.
This commit adds an additional property RejectInfo to the PropertyChanged
signal for org.ofono.ConnectionManager.
So as a rule of thumb, if you're changing the D-Bus API, send the patch
documenting the proposed change against doc/ first, before any commits
implementing this change.
Also note that we do not export vendor specific signals over org.ofono.*
interfaces. If you want to do something vendor specific, implement it in the
modem driver and use org.ofono..* interface prefix. See
plugins/xmm7xxx.c for an example.
---
include/gprs.h | 3 ++
src/gprs.c | 126 +
2 files changed, 129 insertions(+)
diff --git a/include/gprs.h b/include/gprs.h
index 20bdb7a4..92c0d260 100644
--- a/include/gprs.h
+++ b/include/gprs.h
@@ -62,6 +62,9 @@ void ofono_gprs_detached_notify(struct ofono_gprs *gprs);
void ofono_gprs_suspend_notify(struct ofono_gprs *gprs, int cause);
void ofono_gprs_resume_notify(struct ofono_gprs *gprs);
void ofono_gprs_bearer_notify(struct ofono_gprs *gprs, int bearer);
+void ofono_gprs_rejectinfo_notify(struct ofono_gprs *gprs, int plmn,
+ int service_domain, int rat_type,
+ int reject_cause, int reject_type);
struct ofono_modem *ofono_gprs_get_modem(struct ofono_gprs *gprs);
diff --git a/src/gprs.c b/src/gprs.c
index daf2611b..d9e24840 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -136,6 +136,14 @@ struct pri_context {
struct ofono_gprs *gprs;
};
+struct rejectinfo {
+ int plmn;
+ int service_domain;
+ int rat_type;
+ int reject_cause;
+ int reject_type;
+};
+
static void gprs_attached_update(struct ofono_gprs *gprs);
static void gprs_netreg_update(struct ofono_gprs *gprs);
static void gprs_deactivate_next(struct ofono_gprs *gprs);
@@ -2734,6 +2742,124 @@ void ofono_gprs_bearer_notify(struct ofono_gprs *gprs,
int bearer)
"Bearer", DBUS_TYPE_STRING, );
}
+static void append_rejectinfo_properties(const struct rejectinfo *ri,
+ DBusMessageIter *iter)
+{
+ DBusMessageIter variant;
+ DBusMessageIter array;
+ char typesig[5];
+ char arraysig[6];
+ const char *rat;
+ const char *service;
+ const char *reason;
+
+ switch (ri->rat_type) {
+ //GERAN
+ case 0:
+ rat = "gprs";
+ break;
+ //UTRAN
+ case 1:
+ rat = "umts";
+ break;
+ //E-UTRAN
+ case 2:
+ rat = "lte";
+ break;
+ default:
+ rat = "unknown";
+ }
+
+ switch (ri->service_domain) {
+ case 0:
+ service = "cs";
+ break;
+ case 1:
+ service = "ps";
+ break;
+ case 2:
+ service = "cs_ps";
+ break;
+ default:
+ service = "unknown";
+ }
+
+ switch (ri->reject_type) {
+ case 0:
+ reason = "register reject";
+ break;
+ case 1:
+ reason = "authentication failure";
+ break;
+ case 2:
+ reason = "service request reject";
+ break;
+ case 3:
+ reason = "detach request from the network";
+ break;
+ default:
+ reason = "unknown";
+ }
+
+ arraysig[0] = DBUS_TYPE_ARRAY;
+ arraysig[1] = typesig[0] = DBUS_DICT_ENTRY_BEGIN_CHAR;
+ arraysig[2] = typesig[1] = DBUS_TYPE_STRING;
+ arraysig[3] = typesig[2] = DBUS_TYPE_VARIANT;
+ arraysig[4] = typesig[3] = DBUS_DICT_ENTRY_END_CHAR;
+ arraysig[5] = typesig[4] = '\0';
+
+ dbus_message_iter_open_container(iter, DBUS_TYPE_VARIANT,
+ arraysig, );
+
+ dbus_message_iter_open_container(, DBUS_TYPE_ARRAY,
+ typesig, );
+
+ ofono_dbus_dict_append(, "PLMN", DBUS_TYPE_INT32, >plmn);
+ ofono_dbus_dict_append(, "Service", DBUS_TYPE_STRING, );
+ ofono_dbus_dict_append(, "Technology", DBUS_TYPE_STRING, );
+ ofono_dbus_dict_append(, "Cause", DBUS_TYPE_INT32,
+ >reject_cause);
+ ofono_dbus_dict_append(, "Type", DBUS_TYPE_INT32,
+ >reject_type);
+ ofono_dbus_dict_append(, "Reason", DBUS_TYPE_STRING, );
+
+ dbus_message_iter_close_container(, );
+ dbus_message_iter_close_container(iter, );
+}
+
+void ofono_gprs_rejectinfo_notify(struct ofono_gprs *gprs, int plmn,
+
Re: [PATCH] huawei: send restore settings command on startup
Hi Jimmy, On 6/25/20 4:29 AM, Jimmy Gysens wrote: When initializing a Huawei device, send the AT command to restore the default AT settings on device restart. Huawei stores all APN settings, which can cause issues when changing the APN. The AT command makes sure the device starts from a clean state. --- plugins/huawei.c | 4 1 file changed, 4 insertions(+) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] huawei: the AT^SYSCFGEX command supports additional modes
Hi Jimmy, On 6/25/20 4:29 AM, Jimmy Gysens wrote: - LTE and UMTS preferred (acqorder = 0302); AT^SYSCFGEX="0302",4000,2,4,4000 - UMTS and GSM preferred (acqorder = 0201); AT^SYSCFGEX="0201",4000,2,4,4000 For AT^SYSCFG, the modes are not available. --- drivers/huaweimodem/radio-settings.c | 13 + 1 file changed, 13 insertions(+) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] atmodem: set PDP/EPS minimum context ID to 1.
Hi Jimmy, On 6/25/20 4:29 AM, Jimmy Gysens wrote: There are manufacturers, like Huawei, returning profile 0 as the minimum PDP/EPS context ID. Profile 0, however, is the default profile used to register to the LTE network. It contains manufacturer specific settings and should not be created by +CGDCONT. Reference: ETSI TS 127 007: AT command set for User Equipment (UE), section 10.1.0. Yeah, but which version of it ;) I think ETSI versions are lagging a bit behind 3GPP versions and we prefer to use 3GPP 27.007 for historical reasons. --- drivers/atmodem/gprs.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c index 58d4eed3..bcf1eaaf 100644 --- a/drivers/atmodem/gprs.c +++ b/drivers/atmodem/gprs.c @@ -787,6 +787,13 @@ static void at_cgdcont_test_cb(gboolean ok, GAtResult *result, if (found == FALSE) goto error; + /* +* ETSI TS 127 007: profile 0 is manufacturer specific. +* Start from PDP/EPS context ID 1. +*/ + if (min <= 0) + min = 1; + g_at_result_next_range can't really return negative numbers. Not sure why we have it returning gints. So I'd prefer to use min == 0 here. Otherwise, this looks harmless enough, but I wonder if you're breaking some drivers doing this since some might report cid=0 as the default bearer being activated by 'ME PDN ACT' unsolicited result code. Which cid does Huawei report in this case? Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] lte: Use the right D-Bus interface for property change signal
Hi Slava, On 6/16/20 5:40 PM, Slava Monich wrote: --- src/lte.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] cbs: Allow the last CBS fragment to be truncated
Hi Slava, On 6/16/20 5:31 PM, Slava Monich wrote: That does happen in real life. --- src/smsutil.c | 23 +++ src/smsutil.h | 1 + 2 files changed, 16 insertions(+), 8 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] create stk atom for Esim Handling
Hi Shweta,
On 6/11/20 2:06 AM, shweta wrote:
From: Shweta Jain
---
plugins/xmm7xxx.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
This patch does not apply cleanly. In fact, I see three distinct series from
you and it isn't clear in what order I would even apply these. Since it looks
like all these series are related, can you please rebase all patches on top of
git HEAD (for example, via 'git pull --rebase') and resubmit the entire set as a
single series?
diff --git a/plugins/xmm7xxx.c b/plugins/xmm7xxx.c
index 6afc8cb..3959583 100644
--- a/plugins/xmm7xxx.c
+++ b/plugins/xmm7xxx.c
@@ -123,6 +123,8 @@ struct xmm7xxx_data {
int coex_lte_id;
int coex_wlan_id;
int coex_bt_id;
+ ofono_bool_t stk_enable;
+ ofono_bool_t enable_euicc;
};
/* eUICC Implementation */
@@ -1888,7 +1890,10 @@ static void switch_sim_state_status(struct ofono_modem
*modem, int status)
ofono_phonebook_create(modem, 0, "atmodem", data->chat);
data->sms_phonebook_added = TRUE;
}
-
+ break;
+ case 18:
+ data->enable_euicc=TRUE;
+ ofono_warn("Esim State With no Profile %d ", status);
why ofono_warn?
break;
default:
ofono_warn("Unknown SIM state %d received", status);
@@ -2077,15 +2082,24 @@ static void xmm7xxx_pre_sim(struct ofono_modem *modem)
data->sim = ofono_sim_create(modem, OFONO_VENDOR_XMM, "atmodem",
data->chat);
xmm_euicc_enable(modem, data->chat);
+ ofono_stk_create(modem, OFONO_VENDOR_XMM, "atmodem", data->chat);
Generally STK isn't available until the PIN has been entered, and thus should be
available only in post_sim state. Is esim somehow different?
}
static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
{
struct cb_data *cbd = user_data;
ofono_modem_online_cb_t cb = cbd->cb;
+ char * strng = cbd->cb;
What does this do??
+ DBG("");
Hmm, not sure what compiler you're using, but C99 doesn't support mixed
declarations and expressions. Nor does our coding style. Please move this
after all the variable declaration block.
struct ofono_error error;
-
+ struct ofono_modem *modem = cbd->data;
+ struct xmm7xxx_data *data = ofono_modem_get_data(modem);
decode_at_error(, g_at_result_final_response(result));
+ if(data->enable_euicc==TRUE && data->stk_enable==TRUE )
+ {
+ g_at_chat_send(data->chat, "AT+CFUN=16", none_prefix,
+ NULL, NULL, NULL);
+ }
Okay, but this is not following our coding style. Refer to doc/coding-style.txt
for details.
cb(, cbd->data);
}
@@ -2108,8 +2122,10 @@ static void xmm7xxx_set_online(struct ofono_modem *modem, ofono_bool_t online,
data->coex_wlan_id = 0;
g_at_chat_unregister(data->chat, data->coex_bt_id);
data->coex_bt_id = 0;
+ data->stk_enable=FALSE;
}
-
+ else
+data->stk_enable=TRUE;
Coding style is all wrong here...
if (g_at_chat_send(data->chat, command, none_prefix,
set_online_cb, cbd, g_free) > 0) {
if (online)
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 2/2] Remove extra code for Esim Handling
Hi Shweta,
On 6/11/20 2:06 AM, shweta wrote:
From: Shweta Jain
Can you elaborate a bit more why this code is no longer needed? Also, how does
the driver now guarantee that +CUSATP will be received by the stk atom? I
believe this was originally added for those devices that didn't shut down fully
(like USB ones) and kept the sim initialized even when .disable() callback was
called.
If this new device/firmware doesn't need this logic, it might be simpler to just
not pass in the OFONO_VENDOR_XMM (use 0 instead) in plugins/xmm7xxx.c
---
drivers/atmodem/stk.c | 18 +-
1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/drivers/atmodem/stk.c b/drivers/atmodem/stk.c
index b2d2081..18510fe 100644
--- a/drivers/atmodem/stk.c
+++ b/drivers/atmodem/stk.c
@@ -180,7 +180,7 @@ static gboolean at_stk_register(gpointer user)
{
struct ofono_stk *stk = user;
struct stk_data *sd = ofono_stk_get_data(stk);
-
+DBG("");
This code not according to our coding style. Nor is it really related to this
commit description.
g_at_chat_register(sd->chat, "+CUSATP:", phonesim_cusatp_notify,
FALSE, stk, NULL);
@@ -190,25 +190,9 @@ static gboolean at_stk_register(gpointer user)
if (sd->vendor == OFONO_VENDOR_PHONESIM)
g_at_chat_register(sd->chat, "*HCMD:", phonesim_hcmd_notify,
FALSE, stk, NULL);
-
- if (sd->vendor == OFONO_VENDOR_XMM) {
- /* enabling stk*/
- g_at_chat_send(sd->chat, "AT+CFUN=6", none_prefix,
- NULL, NULL, NULL);
- /* Here ofono has missed stk menu proactive command
-* that comes after sim initialization only. Doing a
-* sim reset will enable the stk driver to get the
-* missed +CUSATP notifications.
-*/
- g_at_chat_send(sd->chat, "AT+CFUN=27,1", none_prefix,
- NULL, NULL, NULL);
- }
-
ofono_stk_register(stk);
-
return FALSE;
}
-
This change results in a violation of the coding style as well. See
doc/coding-style.txt for more details.
static int at_stk_probe(struct ofono_stk *stk, unsigned int vendor, void
*data)
{
GAtChat *chat = data;
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 1/1] huawei: use AT^SYSCFG for radio setting operations on 3G only modems
Hi Christophe, On 6/11/20 4:12 AM, Christophe Ronco wrote: AT^SYSCFGEX must be used on LTE Huawei modems to enable LTE support. But some modems (or firmwares?) do not support this command and AT^SYSCFG must be used to get/set radio settings. This has been introduced in commit: 22adf6402c828f8b8cca1b65d8a46ba7792eb787 There is a bug in this commit and AT^SYSCFGEX commands are used even on modems not supporting it. --- drivers/huaweimodem/radio-settings.c | 1 + 1 file changed, 1 insertion(+) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: Weird Droid 4 modem protocol and a way to support it
Hi Tony, All we need to do with the TS 27.010 packets is parse the network strength, operate the voice modem, ack SMS notifications, and pass all the other notifications to the qmimodem to deal with over USB. To me it seems ideal would be simple read/write type functions in drivers/motorolamodem. Is there some preferred existing exaple for doing something like that? Not sure I understand. Example of using ell? See drivers/mbimmodem/*. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2] [qmimodem] Implement data capability bearer notify
Hi Marius, On 6/10/20 8:44 AM, Marius Gripsgard wrote: This implements data capability bearer notify to qmi modem. Since this is included in the serving system response this just adds a new data extraction for dc. --- drivers/qmimodem/gprs.c | 27 +++ drivers/qmimodem/nas.c | 36 drivers/qmimodem/nas.h | 23 +++ 3 files changed, 86 insertions(+) + // DC is optional so only notify on successful extraction + if (extract_dc_info(result, _tech)) + ofono_gprs_bearer_notify(gprs, bearer_tech); + I changed the above C++ comment into C style and applied. Thanks! Regards -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] [qmimodem] Implement data capability bearer notify
Hi Marius,
On 6/9/20 3:21 PM, Marius Gripsgard wrote:
This implements data capability bearer notify to qmi modem.
Since this is included in the serving system response this
just adds a new data extraction for dc.
---
drivers/qmimodem/gprs.c | 29 +
drivers/qmimodem/nas.c | 36
drivers/qmimodem/nas.h | 23 +++
3 files changed, 88 insertions(+)
Looks good, just couple of nitpicks:
diff --git a/drivers/qmimodem/gprs.c b/drivers/qmimodem/gprs.c
index 07adbe9a..0ba24da7 100644
--- a/drivers/qmimodem/gprs.c
+++ b/drivers/qmimodem/gprs.c
@@ -68,6 +68,29 @@ static bool extract_ss_info(struct qmi_result *result, int
*status, int *tech)
return true;
}
+static bool extract_dc_info(struct qmi_result *result, int *bearer_tech)
+{
+ const struct qmi_nas_data_capability *dc;
+ uint16_t len;
+ int i;
+
+ DBG("");
+
+ dc = qmi_result_get(result, QMI_NAS_RESULT_DATA_CAPABILIT_STATUS, );
Should this be CAPABILITY?
+ if (!dc)
+ return false;
+
+ *bearer_tech = -1;
+ for (i = 0; i < dc->cap_count; i++) {
+ DBG("radio tech in use %d", dc->cap[i]);
+
+ *bearer_tech = qmi_nas_cap_to_bearer_tech(dc->cap[i]);
I'm fine with this, but out of curiosity: can there be multiple of these? Are
they sorted somehow?
+ }
+
+ return true;
+}
+
+
No double empty lines please
static void get_lte_attach_param_cb(struct qmi_result *result, void
*user_data)
{
struct ofono_gprs *gprs = user_data;
@@ -188,6 +211,7 @@ static int handle_ss_info(struct qmi_result *result, struct
ofono_gprs *gprs)
struct gprs_data *data = ofono_gprs_get_data(gprs);
int status;
int tech;
+ int bearer_tech;
DBG("");
@@ -209,6 +233,11 @@ static int handle_ss_info(struct qmi_result *result, struct ofono_gprs *gprs)
data->last_auto_context_id = 0;
}
+ if (!extract_dc_info(result, _tech))
+ return -1;
So bearer_tech is pretty much optional, are you sure you want to return an error
here if the extraction fails? Maybe something like:
if (extract_dc_info(result, _tech)
ofono_grps_bearer_notify(gprs, bearer_tech);
would be a bit more conservative.
+
+ ofono_gprs_bearer_notify(gprs, bearer_tech);
+
return status;
}
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: Weird Droid 4 modem protocol and a way to support it
Hi Pavel, On 6/8/20 6:59 PM, Pavel Machek wrote: Hi! I'd really like to get support for Droid 4 modem... unfortunately it is quite special. Few words about Droid 4 modem protocol: I'm not sure what is the best way to support it. I was not able to get atchat.c to work with it (and I don't think it is quite suitable), so I ended up copying it and modifying it for Droid 4 protocol. Is that acceptable? Can you see a better way? I don't think there's really another way. So the approach of duplicating GAtChat and everything inside drivers/atmodem is likely the way to go. But if you pursue this, we really should throw out as much of the legacy in gatchat as possible: - g_at_chat_suspend / resume is likely not needed (you're probably not running PPP over these, right)? - stuff like g_at_chat_set_wakeup is only relevant for some weird modems and probably isn't relevant here - add_terminator / blacklist_terminator might not be needed - Some other concepts might not be needed, like send_pdu_listing and send_and_expect_short_prompt. Those are really only for weird SMS commands. - It might also be possible to greatly simplify the GAtParser concept. - I'd also just put this all directly into drivers/motmodem/* instead of trying to extend gatchat library itself. Most importantly though, we should stop using glib. oFono is (glacially slowly) being ported over to ell. So I don't really want to accept any new glib code. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: Missing documentation for SimManager property writing?
Hi Pavel, On 5/19/20 3:50 AM, Pavel Machek wrote: Hi! In doc/sim-api.txt, there are some properties marked read/write, but I don't see documented way to change those properties. Someone may want to fix that up. Ah thanks for pointing this out. Should now be fixed upstream by commit a88d1120a4b0c82c0368a4457179ee8ca64bd884. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v3] quectel: EC21 needs aux channel to be the first mux channel
Hi Lars, On 5/29/20 7:43 AM, poesc...@lemonage.de wrote: From: Lars Poeschel The Quectel EC21 does only work correctly, if the mux channel used for aux is the first mux channel. It does only put it's URC messages in the first mux channel, so this has to be the aux channel in our case. To be flexible on the mux order we introduce two arrays here, that then contain the initialization data in their needed order. Initialization data is then applied by for-looping over this array. --- plugins/quectel.c | 61 --- 1 file changed, 47 insertions(+), 14 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2] quectel: EC21 needs aux channel to be the first mux channel
Hi Lars,
On 5/28/20 4:32 AM, poesc...@lemonage.de wrote:
From: Lars Poeschel
The Quectel EC21 does only work correctly, if the mux channel used for
aux is the first mux channel. It does only put it's URC messages in the
first mux channel, so this has to be the aux channel in our case.
To be flexible on the mux order we introduce an array here, that
contains the initialization data in it's needed order and is then simply
applied by a for-loop. Initialization of this array is done after we
queried the modem model.
---
plugins/quectel.c | 72 ++-
1 file changed, 58 insertions(+), 14 deletions(-)
So this looks mostly reasonable, but see below:
diff --git a/plugins/quectel.c b/plugins/quectel.c
index 043d39f9..1cad35d6 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -78,6 +78,21 @@ static const uint8_t gsm0710_terminate[] = {
0xf9, /* close flag */
};
+enum mux_type {
+ QUECTEL_MUX_TYPE_AUX = 0,
+ QUECTEL_MUX_TYPE_MODEM,
+ QUECTEL_MUX_TYPE_MAX,
+};
+
+struct mux_initialization_data {
+ enum mux_type mux_type;
+ char *chat_debug;
+ const char *n_gsm_key;
+ const char *n_gsm_value;
+};
+
+static struct mux_initialization_data mux_order[QUECTEL_MUX_TYPE_MAX];
+
So the issue with this is that this driver now no-longer supports
multiple modems of the same type active simultaneously (since all
instances would be trying to write / read from this location).
A better approach would be to use two such variables, i.e.
mux_order_ec21 and mux_order_default and then either store a pointer to
the one to use inside quectel_data, or programmatically by looking up
based on the quectel_data::model.
Alternatively, storing mux_order in quectel_data itself is also an option.
enum quectel_model {
QUECTEL_UNKNOWN,
QUECTEL_UC15,
@@ -1014,6 +1037,17 @@ static void cgmm_cb(int ok, GAtResult *result, void
*user_data)
return;
}
+ mux_order[0] = (struct mux_initialization_data)
+ { QUECTEL_MUX_TYPE_MODEM,
+ "Modem: ",
+ "Modem",
+ "/dev/gsmtty1"};
+ mux_order[1] = (struct mux_initialization_data)
+ { QUECTEL_MUX_TYPE_AUX,
+ "Aux: ",
+ "Aux",
+ "/dev/gsmtty2"};
+
This would then move into the static initializer above...
if (strcmp(model, "UC15") == 0) {
DBG("%p model UC15", modem);
data->vendor = OFONO_VENDOR_QUECTEL;
@@ -1030,6 +1064,16 @@ static void cgmm_cb(int ok, GAtResult *result, void
*user_data)
DBG("%p model EC21", modem);
data->vendor = OFONO_VENDOR_QUECTEL;
data->model = QUECTEL_EC21;
+ mux_order[0] = (struct mux_initialization_data)
+ { QUECTEL_MUX_TYPE_AUX,
+ "Aux: ",
+ "Aux",
+ "/dev/gsmtty1"};
+ mux_order[1] = (struct mux_initialization_data)
+ { QUECTEL_MUX_TYPE_MODEM,
+ "Modem: ",
+ "Modem",
+ "/dev/gsmtty2"};
} else {
ofono_warn("%p unknown model: '%s'", modem, model);
data->vendor = OFONO_VENDOR_QUECTEL;
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 4/7] quectel: EC21 needs aux channel to be the first mux channel
Hi Lars,
On 5/26/20 5:16 AM, poesc...@lemonage.de wrote:
From: Lars Poeschel
The Quectel EC21 does only work correctly, if the mux channel used for
aux is the first mux channel. It does only put it's URC messages in the
first mux channel, so this has to be the aux channel in our case.
---
plugins/quectel.c | 51 +++
1 file changed, 38 insertions(+), 13 deletions(-)
diff --git a/plugins/quectel.c b/plugins/quectel.c
index 1d312c45..9ff75516 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -847,18 +847,38 @@ static void cmux_gatmux(struct ofono_modem *modem)
g_at_mux_start(data->mux);
- data->modem = create_chat(modem, "Modem: ");
- if (!data->modem) {
- ofono_error("failed to create modem channel");
- close_serial(modem);
- return;
- }
+ if (data->model == QUECTEL_EC21) {
+ data->aux = create_chat(modem, "Aux: ");
- data->aux = create_chat(modem, "Aux: ");
- if (!data->aux) {
- ofono_error("failed to create aux channel");
- close_serial(modem);
- return;
+ if (!data->aux) {
+ ofono_error("failed to create aux channel");
+ close_serial(modem);
+ return;
+ }
+
+ data->modem = create_chat(modem, "Modem: ");
+
+ if (!data->modem) {
+ ofono_error("failed to create modem channel");
+ close_serial(modem);
+ return;
+ }
+ } else {
+ data->modem = create_chat(modem, "Modem: ");
+
+ if (!data->modem) {
+ ofono_error("failed to create modem channel");
+ close_serial(modem);
+ return;
+ }
+
+ data->aux = create_chat(modem, "Aux: ");
+
+ if (!data->aux) {
+ ofono_error("failed to create aux channel");
+ close_serial(modem);
+ return;
+ }
}
Can we be smarter about this and just store the per-model creation
sequence as an array or something? Then have a loop that calls
create_chat from the array info? The proposed copy-pasting approach is
not maintainable long term.
setup_aux(modem);
@@ -951,8 +971,13 @@ static void cmux_ngsm(struct ofono_modem *modem)
* the kernel does not yet support mapping the underlying serial device
* to its virtual gsm ttys, so hard-code gsmtty1 gsmtty2 for now
*/
- ofono_modem_set_string(modem, "Modem", "/dev/gsmtty1");
- ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
+ if (data->model == QUECTEL_EC21) {
+ ofono_modem_set_string(modem, "Modem", "/dev/gsmtty2");
+ ofono_modem_set_string(modem, "Aux", "/dev/gsmtty1");
+ } else {
+ ofono_modem_set_string(modem, "Modem", "/dev/gsmtty1");
+ ofono_modem_set_string(modem, "Aux", "/dev/gsmtty2");
+ }
Doesn't this break the logic in mux_ready_cb, particularly the 'check if
the last virtual gsm tty's are created'
/* wait for gsmtty devices to appear */
if (!l_timeout_create_ms(100, mux_ready_cb, modem, NULL)) {
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 0/7] Add quectel EC21 in serial mode
Hi Lars, On 5/26/20 5:16 AM, poesc...@lemonage.de wrote: From: Lars Poeschel This patchset adds support for the quectel EC21 LTE modem connected through its serial port. Lars Poeschel (7): quectel: Add Quectel EC21 to known serial modems quectel: use lte atom on EC21 quectel: Query the model before setting up the mux quectel: EC21 needs aux channel to be the first mux channel quectel: EC21 does not understand AT+QIURC voicecall: Quectel modem do not understand AT+CNAP quectel: EC21 add ussd with atmodem driver drivers/atmodem/voicecall.c | 4 +- plugins/quectel.c | 153 +++- 2 files changed, 101 insertions(+), 56 deletions(-) So I went ahead and applied everything except patch 4. See my comments in the reply to that patch Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] rilmodem: update call direction from the isMT value
Hi, On 4/5/20 6:26 AM, JongSeok Won wrote: oFono cannot determines the call of direction when the voicecall is triggered in rilmodem --- drivers/rilmodem/voicecall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] plugin: provision: create multiple contexts for multiple entries in mbpi
Hi Tarmo, Once a proper database is used, this mostly goes away as an issue. We did add the SPN field to MBPI schema in the olden days. The problem is that nobody actually updated the MBPI database to take advantage of this. Hmm, maybe my memory is faulty (it has been ~9 years now), but I do not see anything in the mbpi DTD file for the SPN. The oFono change was done in 2011 in ef37b3fe4244d823a5dde1a311c0d466ad2e7172. And we did have someone working on extending mbpi, but it seems SPN changes never made it. I no longer recall what happened, exactly. So if the SPN for every duplicate in mbpi was populated, ofono would start provisioning them? That's useful information. Where could one find the details of this schema documented? It is much more likely to, yes. The number of duplicates goes down tremendously, particularly if the db is reasonably up to date. There may be a few extra bits we can also differentiate on as well, and extend the provisioning API for these. See above about the schema. Maybe someone wants to pick up this work again... Sure, after much, much frustration I arrived to the workaround of manually provisioning APNs (which I stole from mbpi!) for each factory installed SIM through Christophe Ronco's file-provision plugin. This plugin is a life-saver, but it certainly falls short of automatic provisioning. That's the same level of sophistication as pppd. And as extra punishment I have to write a network supervisor which orders connman to actually use the service of a new SIM whenever it is replaced - even if it's from the same MNO. But that's a different rant altogether. So, for perspective - saying "mbpi is just not a very good database" helps me none at all when I need to get my widget to go online :) Please handle the duplication, as this is how the mobile networks are built. I can take a patch that enables duplicates as a config / environmental setting. As long as the default is the old behavior. But as I mentioned previously, ConnMan / oFono were not designed to work this way, and so far nobody has been willing to do the work in both projects to change that. I emphasize. Stepping back, I suspect the task of maintaining a clean, accurate provisioning database of every SIM in the world is rather challenging. Perhaps more challenging, than re-designing ofono :) Quite possible. But I would still question whether trying APNs at semi-random is a good strategy. I could pick up a few commodity SIMs from my country, determine the SPN and contribute updates somewhere. Heh, I don't even know which MVNOs here are dead and which are operational. Exactly. In the Meego days Nokia had their own APN database. And it was considered crown jewels / company secret ;) Understandable, considering how much work probably went into keeping it up to date. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] plugin: provision: create multiple contexts for multiple entries in mbpi
Hi Tarmo, I don't see how am I going to solve this. The end user cannot configure the device (there's no user interaction whatsoever). I could not find the mythical Android database at the time (I do now - it's at https://android.googlesource.com/device/sample/+/master/etc/apns-full-conf.xml). It contains many duplicates, because the virtual MNO-s share MCC and MNC-s with the physical ones. That's how the mobile networks are built in the real world. So I was very confused about how to proceed. MVNOs are handled by utilizing the EFspn file as a differentiator. So even if MNC/MCC is the same, once you include the SPN, the number of duplicates goes down drastically. This is why oFono uses the following signature for get_settings: int (*get_settings)(const char *mcc, const char *mnc, const char *spn, Once a proper database is used, this mostly goes away as an issue. We did add the SPN field to MBPI schema in the olden days. The problem is that nobody actually updated the MBPI database to take advantage of this. Sure, after much, much frustration I arrived to the workaround of manually provisioning APNs (which I stole from mbpi!) for each factory installed SIM through Christophe Ronco's file-provision plugin. This plugin is a life-saver, but it certainly falls short of automatic provisioning. That's the same level of sophistication as pppd. And as extra punishment I have to write a network supervisor which orders connman to actually use the service of a new SIM whenever it is replaced - even if it's from the same MNO. But that's a different rant altogether. So, for perspective - saying "mbpi is just not a very good database" helps me none at all when I need to get my widget to go online :) Please handle the duplication, as this is how the mobile networks are built. I can take a patch that enables duplicates as a config / environmental setting. As long as the default is the old behavior. But as I mentioned previously, ConnMan / oFono were not designed to work this way, and so far nobody has been willing to do the work in both projects to change that. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: How to handle modems with additional wwan connect stage?
Hi Mattias, On 3/11/20 7:46 AM, mattias.mans...@verisure.com wrote: We are writing a ofono driver/plugin for a new Quectel modem based on a qualcomm chipset. This modem has a wwan interface that needs to be "connected" after the context is activated, partially to allow the host to run dhcp. It can be done either using QMI or with a special qualcomm command AT$QCRMCALL. We want to control it using AT, as we have found several issues with QMI. But we are having some trouble figuring out where this command should be called in the ofono state machine, at least in the LTE case where the context is automatically activated. If we activate the context manually, we could just add that call in the end of the context activation. But if we for the automatic case add this command when we receive PDN ACT 1 and the AT command fails (which right now happens sometimes), we end up in a state where we can't try again, because the context is already activated. So we can't really find the best way to do this... I guess you would have to build in logic to retry the command as part of the .read_settings operation. In other words, try to retry your AT$MAGIC command several times prior to giving up. Or contact your modem vendor to fix their firmware :) Another issue I wanted to ask is that when we get an automatic context activation on 4G, we don't get the APN set from the network in the AT+CGDCONT? response. According to Quectel, this is just how their modem work. It will only show manually set APN from that response. Is this normal for any other modules? And how to solve the problem that ofono wants the APN when calling ofono_gprs_cid_activated when we get the CGEV saying the PDN is active? By 4G you mean UMTS/UTRAN, not LTE/EUTRAN, right? Not sure anyone really tested UMTS network-activated contexts, all testing was focused on LTE, but in theory they should work just the same. The key difference might be that the default bearer for LTE is usually still defined on the ME, so +CGDCONT would work for those contexts and not for truly dynamic network activated ones. According to 27.007, +CGDCONT is only used to define parameters. For obtaining 'dynamic' parameters, CGCONTRDP should be used instead. Perhaps the AT modem driver should be using CGCONTRDP instead of +CGDCONT when issuing ofono_gprs_cid_activated. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] plugin: provision: create multiple contexts for multiple entries in mbpi
Hi, In my case automatic provisioning always fails: - the database has multiple entries for basically every operator/country mbpi is just not a very good database. It provides lots of duplicates and doesn't distinguish by spn last I checked. Ubuntu Touch folks used the android apndb and others used custom ones. As far as I'm aware, ofono + mbpi was never used in production. If I'm wrong, I'd love to hear about it. I have discouraged the use of mbpi for these reasons. Not stopping you from using it, just pointing out what has been done historically. - also Ofono doesn;t expose a Dbus API to query the database There was never a reason to... - so the current plugin is useless: it always provision the wrong settings Pretty much, yes. If I want to do it externally I have to reimplement the whole "quering thedatabase logic", delete all the automatic provisioned profile and setup custom ones. The null profile created when automatic provisioning fails is by design. It serves as a hint to ConnMan & settings service that manual provisioning is required. So the current behavior is how oFono + ConnMan were designed to interoperate. You can certainly try and change the design, I'm just pointing out that this might not be the path of least resistance. What is the more "elegant" solution in your opinion? Not sure, ideally there should be a way to avoid something like this. Also some providers allow a context to be activated, but no traffic can pass, so this might not be a foolproof solution anyway. mmm Those are details that are exposed in every connection manager that I know of... see Android network settings for example (maybe there's a reason??) That is something you can take up with ConnMan guys. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] plugin: provision: create multiple contexts for multiple entries in mbpi
Hi Nicola, No top posting please. On 3/4/20 11:00 AM, nick83ola wrote: Hi Giacinto, We have a web application that is talking to connman and ofono over dbus. We present the user with the list and once is connected to a profile connman remembers it so if I connect again the same modem connman will remember it as a favourite and automatically connect. or at least is what I'm thinking of doing. If you're making the user choose, why not follow the advice I have already given Martin and others? Just provision the context once if/when automatic provisioning fails. This is how connman/ofono were designed to be used. Any workarounds you introduce will just make your life harder. To automatically connect without the user I was thinking of implementing what you said: - retrieve "cellular" services from connman - check if there are favourite services - if there are not try one context one after the another So method of elimination. Doesn't sound elegant ;) The main issue is that connman is not exposing the APN and data over dbus and is not easy to identify also the context which a service is referring to... the other indication is the name So if I create something in ofono I don't know how connman will call it :-( ConnMan was explicitly designed not to expose such details. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] plugin: provision: create multiple contexts for multiple entries in mbpi
Hi Nicola, On 3/2/20 10:31 AM, Nicola Lunghi wrote: if the mobile provider information database has multiple apn settings for the same operator, ofono was throwing an error and creating a default internet context with an empty apn. This patch will instead allow the automatic creation of multiple context allowing the user to pick one of the default via connman. I still maintain this is a horrible idea. Yes it is a nice workaround if you don't have a proper provisioning database, but not something I'd use in production. What I can do is allow this to be configurable via some config file or environment variable (and off by default). Connman supports multiple cellular context so no issue there. Previously proposed by Martin Hundebøll here: https://lists.ofono.org/hyperkitty/list/ofono@ofono.org/thread/2SC46PH5CWT3A3HTHGUKUUVI3QDYIL73/#7B6CPARJQMZUBQUPXBJMAOXZY4RW2L3D And tested by Nicola Lunghi with connman 1.37 Signed-off-by: Nicola Lunghi No SoB please. --- plugins/provision.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] build: require dbus >= 1.6
Hi Joey, On 3/4/20 1:16 AM, j...@joeyhewitt.com wrote: dbus_validate_path() is used several times. dbus's NEWS says it was added in 1.5.12. --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: Option to not power off radio during start ofono
Hi Richard, On 2/18/20 3:19 AM, Richard Röjfors wrote: Hi, Ofono (at least for ublox) is always powering off the radio during start. This can of course be handy of programmatic reasons to bring the modem to a known state. Some configuration requires the radio to be turned off; for instance the LTE auto connect APN. But on the other hand these are stored in non-volatile memory and could be configured before hand. So by default we bring the device into airplane mode until the SIM card has been initialized. Also, the policy decision of bringing the radio up / down was left to external services such as ConnMan. The big drawback with turning it off is that it might take time to register again when the radio is on. I have seen it taking more than 10 minutes in extreme cases. That's nuts. Shouldn't take more than a few seconds? In embedded systems this can be a big issue. Indeed. I'm thinking of adding a configuration option to keep the radio on during start. Whats the general thought about this? Not sure. Are you certain that adding such low-level config options is the right approach? Even documenting / explaining what and why this configuration option is there would be challenging I imagine. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2] ublox: network-registration: Handle UREG unsolicited during poll
Hi Richard, On 2/14/20 3:06 AM, richard.rojf...@gmail.com wrote: From: Richard Röjfors In the case a unsolicited indication for UREG was received while the status was polled. The poll response failed to parse. This since the unsolicited indication only carries one parameter, while the poll response is expected to carry two. Update the code to loop until the response is found. The log below shows a case where this happened. 10:07:55 ofonod[520]: Aux: > AT+UREG?\r 10:07:55 ofonod[520]: Aux: < \r\n+CGREG: 4\r\n\r\n+UREG: 0\r\n\r\n+CIEV: 9,1\r\n 10:07:55 ofonod[520]: src/gprs.c:ofono_gprs_status_notify() /ublox_0 status unknown (4) 10:07:55 ofonod[520]: src/gprs.c:ofono_gprs_detached_notify() /ublox_0 10:07:55 ofonod[520]: Aux: < \r\n+UREG: 1,0\r\n 10:07:55 ofonod[520]: Aux: < \r\nOK\r\n --- drivers/ubloxmodem/network-registration.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) Applied with a minor whitespace fix. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: HSP/HFP ofono bluetooth support for Linux desktop
Hi Pali, On 2/13/20 12:32 PM, Pali Rohár wrote: At the time this was all done in software. CVSD was never done in software. Always in hardware. As said, even now I was not able to find bluetooth HW which would allow to do CVSD in software. I don't remember the exact details. I seem to remember that for mSBC the conversion was being done on the host and no 'on-the-fly' conversion was done in hardware. Thus this host codec negotiation was not needed / considered. https://lists.ofono.org/hyperkitty/list/ofono@ofono.org/message/6CUFGDPUJBRIZA4GUVFD2EPOET25XTN3/ So how do you negotiate the 'AgentCodec'? Does BlueZ expose this information? If so, how? SCO socket option or ...? It is done by HCI commands, therefore by kernel. There is discussion for exporting userspace <--> kernel API to allow setting arbitrary configurations for codecs supported by bluetooth HW. Getting list of supported codecs can be done by this script: https://github.com/pali/hsphfpd-prototype/blob/prototype/sco_features.pl (needs to be run as root) So you might want to get BlueZ guys to expose this info properly first. oFono is not in the business of opening raw hci sockets. Isn't the MTU obtained from the SCO socket itself? How is oFono at fault here? Yes, via some ioctl it can be done. But bluez for other bluetooth profiles provides this information via dbus API. As bluez does not support HSP/HFP it expects that software which implement it, provide needed info Only PA (or whatever implements the audio agent) really cares about this info and it can obtain it via getsockopt. So I really don't see why the MTU should be exposed via D-Bus. And this is why it wasn't. I don't see an issue here...? This seems to be a kernel / device driver / firmware issue and should be solved at that level. Why??? It is up to the application which owns SLC socket and this application needs to provide API for it. Codecs are negotiated via AT commands, so again only HFP / HSP daemon can do it. So in my opinion it is really up to the kernel to tell us whether a given hardware supports wideband speech. So any quirks need to go into the kernel. Then userspace can select the best available codec automatically without resorting to having the user twiddle some settings. So, why should I even consider to use ofono at all? It does not support none of above desktop feature, it does not support extended codecs, it does not support HSP profile and also it does not support HFP profile without physical modem (majority of desktops and laptops). Your initial proposal wanted to use oFono as some sort of helper for your daemon, and that is just not going to be accepted by oFono upstream. I gave you a few alternatives, including how to extend oFono to do what you want. If you want to roll your own, go for it. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: HSP/HFP ofono bluetooth support for Linux desktop
Hi Pali, Used by who? Gateway role is fully broken and client (hfp) role is used I guess that depends on your perspective. I've already pointed out that the desktop 'AG' use case was never something we needed to implement. If you want to fix oFono to do that, great. If you want to write your own daemon to do this, also great. probably only by some power users. Also there is no support for mSBC in pusleaudio. Why is oFono at fault for this? Genuine question. What are the roadblocks to mSBC support? So, no, really it is not used. Main objection for handsfree-audio-api.txt is that it does not provide information about locally used codec and somehow mixes air codec and local codec. In my hsphfpd.txt I used term "AgentCodec" for bluetooth local codec and term "AirCodec" for bluetooth air codec format. Okay. But, just FYI, at the time there was no hw that could do such on-the-fly conversions, so this use case wasn't considered/implemented. This cannot be truth as probably every bluetooth HW is doing on-the-fly conversion between CVSD and PCM. I was not able to find HW which allows me to send raw CVSD samples... At the time this was all done in software. Alternatively, the hardware was directly wired between the sound card / modem and the bluetooth chip. So just opening the SCO socket was enough. True. In retrospect we probably should have used strings. But it was assumed that vendor extensions would go via the Bluetooth SIG Assigned Numbers facility. Anyhow, we can always add a 'Register2' method that could take codecs as a string array or a combination of strings & ints. And if Register2 was used, then use 'NewConnection2' with a signature that supports passing in vendor codecs and whatever else that might be needed. This is still not enough. Audio application (e.g. pulseaudio) need to register AgentCodec, not AirCodec. And current API is somehow mixed. Audio application needs to know what is the format which bluetooth chip sends to userspace (PCM? mSBC? CVSD? PCMA? AuriStream?) and which format bluetooth chip expects. I named this AgentCodec. So how do you negotiate the 'AgentCodec'? Does BlueZ expose this information? If so, how? SCO socket option or ...? And also API does not provide socket MTU information or ability to change/specify which codec would be used. There was no need, we automatically defaulted to using Wide band if available. Third party codecs are a new use case (for oFono HFP), so would require an API extension. MTU is needed also for mSBC codec if encoding is done in software (pulseaudio). Without it, this wide band support in ofono is unusable for pulseaudio. Isn't the MTU obtained from the SCO socket itself? How is oFono at fault here? And also API extension for choosing codec. Also for choosing if software of hardware encoding would be used. We know that there are lot of broken devices in different way and it is really needed for either blacklist some codec or switch between hw and sw encoding if something strange happen. Without it pulseaudio is not going to support more codes then default required (CVSD). This seems to be a kernel / device driver / firmware issue and should be solved at that level. Non-audio APIs which are needed to export (for both HSP and HFP profiles): * battery level (0% - 100%) * power source (external, battery, unknown) * ability to send "our laptop" battery level and power source to remote device * send text message to embedded display * process button press event (exported via linux kernel uinput) I think all of these are feasible to support under the current oFono structure, either via plugins or API extensions. Ok. Are you going to implement them? I think that all of these are missing parts in ofono and something which is needed to be implemented for desktop/laptop HSP and HFP profile support. There are no plans to do this at the moment. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: HSP/HFP ofono bluetooth support for Linux desktop
Hi Pali, On 1/8/20 3:25 PM, Pali Rohár wrote: Hello! Somehow this went straight to my Junk folder, so I didn't see this message at all until now. Audio application (e.g. pulseaudio) really do not want to handle two separate services to monitor and process HSP/HFP devices. > For audio application are HSP and HFP devices equivalent, they provide same features: SCO socket, API for controlling microphone and speaker gain; plus optionally specify used codec. So having two separate services which fully divided for audio application purpose does not make sense. So it is possible that both HSP and HFP audio cards would be available via one audio API? Because I do not see how it could be done via design like dundee. One API sure. Maybe on multiple services. Honestly, I don't see why this would be such a burden for PA to watch 2 dbus services instead of 1. From oFono perspective it would make more sense to keep the HSP part a separate daemon. I could be convinced otherwise if it is indeed a big burden for PA... You can then implement the same API interfaces for setting up the HSP audio stream as oFono does today (i.e. https://git.kernel.org/pub/scm/network/ofono/ofono.git/tree/doc/handsfree-audio-api.txt), This API is unusable for both HSP and HFP audio streams. In pulseaudio it is somehow used, but it is not suitable. Funny. "It is used but not suitable"? Main objection for handsfree-audio-api.txt is that it does not provide information about locally used codec and somehow mixes air codec and local codec. In my hsphfpd.txt I used term "AgentCodec" for bluetooth local codec and term "AirCodec" for bluetooth air codec format. Okay. But, just FYI, at the time there was no hw that could do such on-the-fly conversions, so this use case wasn't considered/implemented. There's really no reason why we couldn't extend our APIs to handle this. Another objection against handsfree-audio-api.txt API is that it is bound to HF codecs (via number) and does not support for pass e.g. CSR codecs. True. In retrospect we probably should have used strings. But it was assumed that vendor extensions would go via the Bluetooth SIG Assigned Numbers facility. Anyhow, we can always add a 'Register2' method that could take codecs as a string array or a combination of strings & ints. And if Register2 was used, then use 'NewConnection2' with a signature that supports passing in vendor codecs and whatever else that might be needed. What is completely missing in that API is controlling volume level. It is there on the CallVolume interface And also API does not provide socket MTU information or ability to change/specify which codec would be used. There was no need, we automatically defaulted to using Wide band if available. Third party codecs are a new use case (for oFono HFP), so would require an API extension. Non-audio APIs which are needed to export (for both HSP and HFP profiles): * battery level (0% - 100%) * power source (external, battery, unknown) * ability to send "our laptop" battery level and power source to remote device * send text message to embedded display * process button press event (exported via linux kernel uinput) I think all of these are feasible to support under the current oFono structure, either via plugins or API extensions. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: AuthenticationMethod can't be set to none
Hi, Additionnal informations: versions: ``` # ofonod -v 1.24 The 'none' authentication method type was added in oFono 1.26. So your version is a bit outdated. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: org.ofono.AllowedAccessPoints not found on the interface_list
Hi JH,
On 1/24/20 6:49 PM, JH wrote:
Hi,
I am running ofono for SARA-R4 Modem, systemd show many following
messages, does it need be concerned?
Interface org.ofono.AllowedAccessPoints not found on the interface_list
This error message should be benign. See if the attached patch quiets
this down for you.
Regards,
-Denis
>From 8e78d4dba5d54b4b1175a53973ceb6f829c22bfa Mon Sep 17 00:00:00 2001
From: Denis Kenzior
Date: Fri, 7 Feb 2020 11:06:32 -0600
Subject: [PATCH] allowed-apns: Do not try to unregister unnecessarily
allowed-apns plugin will try to uregister the AllowedAccessPoints
interface whenever the sim state changes, even when not registered.
This results in the (benign) error being printed inside
ofono_modem_remove_interface:
Interface org.ofono.AllowedAccessPoints not found on the interface_list
---
plugins/allowed-apns.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/plugins/allowed-apns.c b/plugins/allowed-apns.c
index b222b91c..199202b5 100644
--- a/plugins/allowed-apns.c
+++ b/plugins/allowed-apns.c
@@ -52,6 +52,7 @@ struct allowed_apns_ctx {
struct ofono_sim_context *sim_context;
DBusMessage *pending;
DBusMessage *reply;
+ bool registered;
};
static void context_destroy(gpointer data)
@@ -162,6 +163,9 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *data)
DBusConnection *conn = ofono_dbus_get_connection();
if (new_state != OFONO_SIM_STATE_READY) {
+ if (!ctx->registered)
+ return;
+
g_dbus_unregister_interface(conn,
ofono_modem_get_path(ctx->modem),
ALLOWED_ACCESS_POINTS_INTERFACE);
@@ -169,6 +173,7 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *data)
ofono_modem_remove_interface(ctx->modem,
ALLOWED_ACCESS_POINTS_INTERFACE);
+ ctx->registered = false;
return;
}
@@ -183,6 +188,7 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *data)
return;
}
+ ctx->registered = true;
ofono_modem_add_interface(ctx->modem,
ALLOWED_ACCESS_POINTS_INTERFACE);
}
--
2.21.0
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] ublox: netreg: reuse at_registration_status
Hi Richard, On 2/4/20 6:21 AM, richard.rojf...@gmail.com wrote: From: Richard Röjfors Instead of implementing an own copy of requesting and parsing CREG, reuse the existing one from at-modem. --- drivers/ubloxmodem/network-registration.c | 40 +-- 1 file changed, 16 insertions(+), 24 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: Where are ofono dbus paths for RSRQ and RSRP?
Hi, Sorry, I am still learning dbus command and syntax, can the NetworkMonitor interface be run via dbus-send? It can... Using dbus-send is not too user-friendly though. I'd suggest a UI based tool. Something like d-feet, or qdbusviewer. dict entry( string "Interfaces" variant array [ string "org.ofono.SmartMessaging" string "org.ofono.PushNotification" string "org.ofono.MessageManager" string "org.ofono.LongTermEvolution" string "org.ofono.NetworkRegistration" string "org.ofono.RadioSettings" string "org.ofono.ConnectionManager" string "org.ofono.NetworkMonitor" So the modem repots NetworkMonitor support... string "org.ofono.MessageWaiting" string "org.ofono.AllowedAccessPoints" string "org.ofono.SimManager" ] ) But I got following command syntax error, what is the correct syntax to run NetworkMonitor? # dbus-send --print-reply --system --dest=org.ofono /ubloxqmi_0 org.ofono.NetworkMonitor.GetProperties This won't work because NetworkMonitor interface does not have Properties. Refer to the networkmonitor-api.txt documentation: https://git.kernel.org/pub/scm/network/ofono/ofono.git/tree/doc/networkmonitor-api.txt You can invoke the org.ofono.NetworkMonitor.GetServingCellInformation or GetNeighbouringCellsInformation methods and see if this particular value would be reported. If you're on uBlox hardware, then i think it only reports the Serving Cell info, not neighbor cells. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: Where are ofono dbus paths for RSRQ and RSRP?
Hi, On 1/21/20 7:56 PM, JH wrote: Hi, I have been looking for getting RSRQ and RSRP values from ofono dbus commands, but I could not find much information how to get RSRQ and RSRP in debus-send commands, which following interface contains RSRQ and RSRP? And how could I access thos RSRQ and RSRP in debus-send? What exactly is RSRP and RSRQ? Are you referring to ReferenceSignalReceivedQuality and ReferenceSignalReceivedPower on LTE? If so, these would be available on the NetworkMonitor interface assuming your modem / driver reports these. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
test please ignore
___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: HSP/HFP ofono bluetooth support for Linux desktop
Hi Pali, Do you have a reasonable solution also for second issue? HSP profile has always been a problem child. It isn't really all that useful as a profile, and given how minimal it is, the right place for it always seemed to be inside Pulse Audio itself. This is what Marcel & I agreed upon back about 8-9 years ago anyway. You are advocating that HSP is still useful, particularly with vendor extensions. Which is fair enough, but now you have to figure out how and where to put this support. As mentioned earlier, one idea you can explore is to create a small daemon (or maybe it can even be part of ofonod itself) that will handle HSP client/server roles. See for example the dundee daemon that is part of ofono.git. dundee handles Bluetooth DUN profile and might be a good model / starting point for what you're trying to accomplish. You can then implement the same API interfaces for setting up the HSP audio stream as oFono does today (i.e. https://git.kernel.org/pub/scm/network/ofono/ofono.git/tree/doc/handsfree-audio-api.txt), which would make PulseAudio's job much easier, since the audio stream aspects would be essentially identical to HFP. If you're part of oFono's tree, then in theory many implementation aspects could be reused. If you want to provide some higher-level APIs for external applications, then HSP specific interfaces (APIs) can be added as needed. If you decide this is something you want to pursue, then I'm happy to host this in the oFono tree. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] xmm7modem: CPIN handling after sending puk
Hi Antara, On 12/19/19 6:57 AM, Antara Borwankar wrote: On XMM modems SIM is busy after PUK is entered. CME ERROR: 14 is received for AT+CPIN? query. Therefore polling for CPIN: READY state. --- drivers/atmodem/sim.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c index e750a13..3ed5aa0 100644 --- a/drivers/atmodem/sim.c +++ b/drivers/atmodem/sim.c @@ -1354,13 +1354,14 @@ static void at_pin_send_cb(gboolean ok, GAtResult *result, case OFONO_VENDOR_HUAWEI: case OFONO_VENDOR_SIMCOM: case OFONO_VENDOR_SIERRA: + case OFONO_VENDOR_XMM: /* * On ZTE modems, after pin is entered, SIM state is checked * by polling CPIN as their modem doesn't provide unsolicited * notification of SIM readiness. * -* On SIMCOM modems, SIM is busy after pin is entered (we -* got a "+CME ERROR: 14" for the "AT+CPIN?" request) and +* On SIMCOM and XMM modems, SIM is busy after pin is entered +* (we got a "+CME ERROR: 14" for the "AT+CPIN?" request) and * ofono don't catch the "+CPIN: READY" message sent by the * modem when SIM is ready. So, use extra CPIN to check the * state. Shouldn't this be already taken care of by setting wait_initialized inside src/sim.c sim_enter_pin_cb()? This particular logic is a work around for modems that do not have a separate notification that can be used to send ofono_sim_initialized_notify. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] sim: handling crash in error scenario for SIM PIN query
Hi Antara, On 12/19/19 6:57 AM, Antara Borwankar wrote: In case of error in sim_pin_query_cb function. pin_type is set to -1. This is causing segmentation fault in function sim_passwd_name due to invalid index pin_type = -1. Fixing this issue by handling error case before calling sim_passwd_name function. --- src/sim.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] xmm7xxx: modified handling of XSIM states for xmm modems
Hi Antara, On 12/19/19 6:56 AM, Antara Borwankar wrote: +XSIM:7 state as defined in xmm7560 functional AT specification only indicates ready for attach. +CPIN: READY is received after SIM is completely initialized. Also indicating readiness of Phonebook and SMS. Hence moving the creation of SMS and Phonebook atom to xmm7xxx_post_sim function. +XSIM:4 PUK needed state was not handled. It must be handled same as PIN needed state. Added handling of this case to switch_sim_state_status function. --- plugins/xmm7xxx.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] xmm7xxx: handling of sms ready state for xmm7xxx plugin
Hi Antara,
On 12/16/19 4:37 AM, Antara Borwankar wrote:
+XSIM:7 state as defined in xmm7560 functional AT specification
Is this different compared to earlier models?
only indicates ready for attach. PB ready and SMS ready has to be
quired seperately using +XSIMSTATE command after +XSIM:12 state
typos? queried and separately?
is received indicating SIM SMS Caching Completed. (Sent only when
SMS caching enabled). +XSIM:12 may or may not be received so moving
the sms ready and pb ready logic to post sim function afteR receiving
+CPIN:READY
---
plugins/xmm7xxx.c | 72 ++-
1 file changed, 61 insertions(+), 11 deletions(-)
diff --git a/plugins/xmm7xxx.c b/plugins/xmm7xxx.c
index 32c024e..f62a093 100644
--- a/plugins/xmm7xxx.c
+++ b/plugins/xmm7xxx.c
@@ -106,7 +106,8 @@ struct xmm7xxx_data {
GAtChat *chat; /* AT chat */
struct ofono_sim *sim;
ofono_bool_t have_sim;
- ofono_bool_t sms_phonebook_added;
+ ofono_bool_t phonebook_added;
+ ofono_bool_t sms_added;
unsigned int netreg_watch;
};
@@ -968,6 +969,59 @@ static GAtChat *open_device(struct ofono_modem *modem,
NULL);
}
+static void xsimstate_sms_ready_query_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct xmm7xxx_data *data = ofono_modem_get_data(modem);
+ int sms_ready, pb_ready;
+ GAtResultIter iter;
+
+ DBG("%p", modem);
+
+ if (!ok)
+ return;
+
+ g_at_result_iter_init(, result);
+
+ if (!g_at_result_iter_next(, "+XSIMSTATE:"))
+ return;
+
+ if (!g_at_result_iter_skip_next())
+ return;
+
+ if (!g_at_result_iter_skip_next())
+ return;
+
+ if (!g_at_result_iter_next_number(, _ready))
+ return;
+
+ if (!g_at_result_iter_next_number(, _ready))
+ return;
+
+ DBG("sms_ready=%d\n", sms_ready);
+
+ DBG("data->sms_added=%d\n", data->sms_added);
+
+ if (pb_ready && data->phonebook_added == FALSE) {
+ ofono_phonebook_create(modem, 0, "atmodem", data->chat);
+ data->phonebook_added = TRUE;
+ }
+
+ if (sms_ready && data->sms_added == FALSE) {
+ ofono_sms_create(modem, 0, "atmodem", data->chat);
+ data->sms_added = TRUE;
+ }
+}
+
Ok, but it seems to me like this needs to be polled? What if the
initial query fails to report sms or phonebook as ready?
Can you use +PBREADY and whatever the SMS equivalent is instead?
+static void xmm7xxx_get_sms_ready_state(struct ofono_modem *modem)
+{
+ struct xmm7xxx_data *data = ofono_modem_get_data(modem);
+
+ g_at_chat_send(data->chat, "AT+XSIMSTATE?", xsimstate_prefix,
+ xsimstate_sms_ready_query_cb, modem, NULL);
+}
+
static void switch_sim_state_status(struct ofono_modem *modem, int status)
{
struct xmm7xxx_data *data = ofono_modem_get_data(modem);
@@ -980,7 +1034,8 @@ static void switch_sim_state_status(struct ofono_modem
*modem, int status)
if (data->have_sim == TRUE) {
ofono_sim_inserted_notify(data->sim, FALSE);
data->have_sim = FALSE;
- data->sms_phonebook_added = FALSE;
+ data->phonebook_added = FALSE;
+ data->sms_added = FALSE;
}
break;
case 1: /* SIM inserted, PIN verification needed */
@@ -991,20 +1046,13 @@ static void switch_sim_state_status(struct ofono_modem
*modem, int status)
break;
case 2: /* SIM inserted, PIN verification not needed - READY */
case 3: /* SIM inserted, PIN verified - READY */
- case 7: /* SIM inserted, SMS and phonebook - READY */
+ case 7: /* SIM inserted, READY for ATTACH - READY */
if (data->have_sim == FALSE) {
ofono_sim_inserted_notify(data->sim, TRUE);
data->have_sim = TRUE;
}
ofono_sim_initialized_notify(data->sim);
-
- if (data->sms_phonebook_added == FALSE) {
- ofono_phonebook_create(modem, 0, "atmodem", data->chat);
- ofono_sms_create(modem, 0, "atmodem", data->chat);
- data->sms_phonebook_added = TRUE;
- }
-
If you can't use +PBREADY / SMS equivalent, can you kick off the
XSIMSTATE query/polling here...?
break;
default:
ofono_warn("Unknown SIM state %d received", status);
@@ -1083,7 +1131,8 @@ static void cfun_enable_cb(gboolean ok, GAtResult
*result, gpointer user_data)
g_at_chat_send(data->chat, "AT", NULL, NULL, NULL, NULL);
data->have_sim = FALSE;
-
Re: [PATCH] gprs: Update attach state on context deactivation for LTE
Hi Richard, On 12/11/19 1:13 PM, richard.rojf...@gmail.com wrote: From: Richard Röjfors To be considered attached on LTE a context should be activated. But in case the context got deactivated we did not update the attached state, it remained attached. That caused the connection manager to try to re-activate the context manually, but for LTE thats done automatically. In the case of ublox it returns errors, which is passed on to the connection manager, which tries again and again, until we get attached again. It looked like this: 12:03:18 ofonod[547]: Aux: < \r\n+CIEV: 2,3\r\n 12:03:23 ofonod[547]: Aux: < \r\n+CIEV: 2,2\r\n Deactivated Shouldn't we get a CREG actually? That tells us we're not connected at all? Or does the modem somehow still have another default bearer active? Anyhow, I'll go ahead and take this. Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] gprs: Don't modify the context if assign fails
Hi Richard, On 12/10/19 6:58 AM, richard.rojf...@gmail.com wrote: From: Richard Röjfors Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] - oFono v1.31 stuck in loop when unplugging a USB dongle connected via PPP
Hi Jimmy, On 11/19/19 4:53 AM, Jimmy Gysens wrote: Hi, After unplugging a Huawei USB dongle, the 'atoms' in oFono are removed via 'flush_atoms'. Every atom has a destruct function pointer, used as a - yes - destructor. When unplugging a dongle, connected via PPP, oFono will remove the current GPRS context (= data connection). The function calls are: flush_atoms -> destruct -> gprs_context_remove -> at_gprs_context_remove -> modem_disconnect Because the device is physically removed, the IO channel for the AT port is gone. In 'at_gprs_context_remove', there is an attempt to resume communication over that AT port, but that is not possible. This is detected, and 'io_disconnect' (pointer to 'modem_disconnect') is called. 'modem_disconnect' has the same atom and tries to remove it again, so it calls the same destructor. modem_disconnect -> destruct -> gprs_context_remove -> at_gprs_context_remove -> modem_disconnect And the loop is started. The easiest way to fix this, is to move the GPRS context removal. After a disconnect, oFono tries to re-open the AT port. If that fails (and it will in this case), we're done, since 'modem_disconnect' returns. In case the AT port can be opened, for example, when setting up a new connection, the previous GPRS context is removed. The old GPRS context is removed via: - 'flush_atoms' in case the USB device is physically removed. - 'modem_disconnect' in case a new connection is being set up. The loop hereby is fixed, without altering general functionality of oFono. This fix is limited to Huawei devices and has been tested using the following devices: - E3531i-2 - E3372 - E3531s-2 - E369 - E1552 The patch itself: --- ofono-1.31/plugins/huawei.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) Thanks for the detailed report! I went ahead and applied this fix upstream after massaging the commit description a bit. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] test: make all files executable
Hi David, On 11/17/19 4:03 PM, David Lechner wrote: This sets the executable bit on the only two files in the test directory that do not already have it set. --- test/set-sms-alphabet | 0 test/test-serving-cell-info | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 test/set-sms-alphabet mode change 100644 => 100755 test/test-serving-cell-info Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim 12/13] Annotate overriding functions with override
Hi Jonah,
On 11/12/19 3:37 PM, Jonah Brüchert wrote:
* Allows us to notice when a function becomes source incompatible in a Qt major
release,
and our implementation stops overriding.
---
src/aidapplication.cpp | 3 ++-
src/aidapplication.h| 4 ++--
src/callmanager.h | 2 +-
src/control.h | 18 +++
src/hardwaremanipulator.cpp | 1 +
src/phonesim.h | 16 +++---
src/qatutils.cpp| 44 ++---
src/qgsmcodec.h | 10 -
src/qwsppdu.cpp | 6 ++---
src/qwsppdu.h | 4 ++--
src/simapplication.h| 20 -
src/simauth.h | 2 +-
src/simfilesystem.h | 4 ++--
13 files changed, 68 insertions(+), 66 deletions(-)
diff --git a/src/aidapplication.cpp b/src/aidapplication.cpp
index 807d7f9..395fe99 100644
--- a/src/aidapplication.cpp
+++ b/src/aidapplication.cpp
@@ -190,7 +190,8 @@ bool AidAppWrapper::command( const QString& cmd )
return true;
}
-break;
+// if CMD_TYPE_GSM_AUTH is the case, we already returned
+// and can't fallthrough, so no break here
This doesn't seem to belong in this patch, so I left it out.
case CMD_TYPE_UMTS_AUTH:
{
enum UmtsStatus status;
diff --git a/src/hardwaremanipulator.cpp b/src/hardwaremanipulator.cpp
index ea895e7..63c498a 100644
--- a/src/hardwaremanipulator.cpp
+++ b/src/hardwaremanipulator.cpp
@@ -363,4 +363,5 @@ void HardwareManipulator::simAppAbort()
void HardwareManipulator::callManagement( QList *info )
{
+Q_UNUSED(info)
}
Same here
Rest applied, thanks.
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim 13/13] Use bool literals
Hi Jonah, On 11/12/19 3:37 PM, Jonah Brüchert wrote: Done using the modernize-use-bool-literals of clang-tidy --- src/qsmsmessage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim 11/13] Use dynamic_cast where appropriate
Hi Jonah, On 11/12/19 3:37 PM, Jonah Brüchert wrote: Done using the cppcoreguidelines-pro-type-cstyle-cast check of clang-tidy --- src/aidapplication.cpp | 2 +- src/phonesim.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim 10/13] Port from c-style casts to static_cast<>()
Hi Jonah,
On 11/12/19 3:37 PM, Jonah Brüchert wrote:
Done using the google-readability-casting check of clang-tidy
---
src/callmanager.cpp | 6 +-
src/hardwaremanipulator.cpp | 12 +-
src/phonesim.cpp | 62 +++---
src/qatresult.cpp| 10 +-
src/qatresultparser.cpp | 6 +-
src/qatutils.cpp | 116 +--
src/qcbsmessage.cpp | 6 +-
src/qgsmcodec.cpp| 54 ++---
src/qsimcommand.cpp | 370 +--
src/qsimcontrolevent.cpp | 16 +-
src/qsimenvelope.cpp | 48 ++---
src/qsimterminalresponse.cpp | 98 +-
src/qsmsmessage.cpp | 208 ++--
src/qsmsmessage_p.h | 4 +-
src/qwsppdu.cpp | 94 -
src/simapplication.cpp | 68 +++
src/simauth.cpp | 12 +-
src/simfilesystem.cpp| 24 +--
18 files changed, 605 insertions(+), 609 deletions(-)
This patch is really way too long, and since it was seemingly done by
machine, you probably need to check whether all the changes make sense
first. Just a few examples, picking at random:
diff --git a/src/callmanager.cpp b/src/callmanager.cpp
index 1fa13cf..5436e08 100644
--- a/src/callmanager.cpp
+++ b/src/callmanager.cpp
@@ -203,8 +203,8 @@ bool CallManager::command( const QString& cmd )
multiparty = 0;
QString line =
"+CLCC: " + QString::number(info.id) + "," +
-QString::number((int)(info.incoming)) + "," +
-QString::number((int)(info.state)) + ",0," +
+QString::number(static_cast(info.incoming)) + "," +
+QString::number(static_cast(info.state)) + ",0," +
These casts are not needed at all due to implicit conversions
QString::number(multiparty);
if ( !info.number.isEmpty() ) {
line += ",";
@@ -2501,8 +2501,8 @@ void DemoSimApplication::BIPMenu( const
QSimTerminalResponse& resp )
destAddress += 0x21;
destAddress += 0x7F; /* local host 127.0.0.1 */
-destAddress += (char) 0x00;
-destAddress += (char) 0x00;
+destAddress += static_cast(0x00);
+destAddress += static_cast(0x00);
Why use a cast here but not lines above or below..
destAddress += 0x01;
apn += 0x06;
diff --git a/src/simauth.cpp b/src/simauth.cpp
index 93b8826..b5fc7b1 100644
--- a/src/simauth.cpp
+++ b/src/simauth.cpp
@@ -52,8 +52,8 @@ void SimAuth::gsmAuthenticate( QString rand, QString ,
comp128( ki, _rand, _sres, _kc );
-sres = QByteArray( (const char *)_sres, 4 ).toHex();
-kc = QByteArray( (const char *)_kc, 8 ).toHex();
+sres = QByteArray( reinterpret_cast(_sres), 4 ).toHex();
+kc = QByteArray( reinterpret_cast(_kc), 8 ).toHex();
This likely needs a toUtf8 or something instead of messing with casts.
}
Perhaps you want to split it up a bit more so the parts that make sense
can get merged, and the rest we can address piecemeal by removing the
unnecessary casts.
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim 04/13] Replace QtScript with QtQml
Hi Jonah, On 11/12/19 3:37 PM, Jonah Brüchert wrote: --- configure.ac| 2 +- src/control.cpp | 18 +- src/control.h | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) Patches 4-9 applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim 03/13] Port old-style connects
Hi Jonah,
On 11/12/19 3:37 PM, Jonah Brüchert wrote:
* Update CXX Standard to 14 to be able to use qOverload()
---
configure.ac| 4 +--
src/callmanager.cpp | 22 ++---
src/control.cpp | 77 +
src/phonesim.cpp| 56 +
src/phonesim.h | 3 +-
5 files changed, 80 insertions(+), 82 deletions(-)
@@ -527,35 +528,36 @@ SimRules::SimRules(qintptr fd, QObject *p, const
QString& filename, HardwareMan
machine = hmf->create(this, nullptr);
if (machine) {
-connect(machine, SIGNAL(unsolicitedCommand(QString)),
-this, SLOT(unsolicited(QString)));
-connect(machine, SIGNAL(command(QString)),
-this, SLOT(command(QString)));
-connect(machine, SIGNAL(variableChanged(QString,QString)),
-this, SLOT(setVariable(QString,QString)));
-connect(machine, SIGNAL(switchTo(QString)),
-this, SLOT(switchTo(QString)));
+connect(machine, ::unsolicitedCommand,
+this, ::unsolicited);
+connect(machine, ::command,
+this, ::command);
+connect(machine, ::variableChanged,
+this, ::setVariable);
+connect(machine, ::switchTo,
+this, ::switchTo);
}
+
I squashed this stray whitespace...
_callManager = new CallManager(this);
-connect( _callManager, SIGNAL(send(QString)),
- this, SLOT(respond(QString)) );
-connect( _callManager, SIGNAL(unsolicited(QString)),
- this, SLOT(unsolicited(QString)) );
-connect( _callManager, SIGNAL(dialCheck(QString,bool&)),
- this, SLOT(dialCheck(QString,bool&)) );
+connect( _callManager, ::send,
+ this, qOverload( ::respond ) );
+connect( _callManager, ::unsolicited,
+ this, ::unsolicited );
+connect( _callManager, ::dialCheck,
+ this, ::dialCheck );
if ( machine ) {
-connect( machine, SIGNAL(startIncomingCall(QString,QString,QString)),
- _callManager,
SLOT(startIncomingCall(QString,QString,QString)) );
-connect ( _callManager, SIGNAL( callStatesChanged( QList * )
),
- machine, SLOT( callManagement( QList * ) ) );
-connect ( machine, SIGNAL( stateChangedToAlerting() ), _callManager,
-SLOT( dialingToAlerting() ) );
-connect ( machine, SIGNAL( stateChangedToConnected() ), _callManager,
-SLOT( dialingToConnected() ) );
-connect ( machine, SIGNAL( stateChangedToHangup( int ) ), _callManager,
-SLOT( hangupRemote( int ) ) );
+connect( machine, ::startIncomingCall,
_callManager,
+ qOverload(
::startIncomingCall ) );
+connect ( _callManager, ::callStatesChanged,
+ machine, ::callManagement);
+connect ( machine, ::stateChangedToAlerting,
_callManager,
+::dialingToAlerting );
+connect ( machine, ::stateChangedToConnected,
_callManager,
+::dialingToConnected );
+connect ( machine, ::stateChangedToHangup,
_callManager,
+::hangupRemote );
}
connect(this,SIGNAL(readyRead()),
... and applied this patch.
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim 01/13] Use nullptrs
Hi Jonah, On 11/12/19 3:37 PM, Jonah Brüchert wrote: Done using the modernize-use-nullptr check of clang-tidy --- src/aidapplication.h | 2 +- src/callmanager.h | 2 +- src/conformancesimapplication.cpp | 2 +- src/control.cpp | 2 +- src/control.h | 2 +- src/gsmspec.cpp | 20 ++-- src/hardwaremanipulator.h | 2 +- src/main.cpp | 6 ++-- src/phonesim.cpp | 54 +++ src/phonesim.h| 2 +- src/qatresult.cpp | 2 +- src/qatresultparser.cpp | 8 ++--- src/qatutils.cpp | 12 +++ src/qcbsmessage.cpp | 2 +- src/qsmsmessage.cpp | 8 ++--- src/qsmsmessage_p.h | 4 +-- src/qwsppdu.cpp | 4 +-- src/server.cpp| 4 +-- src/simapplication.cpp| 22 ++--- src/simapplication.h | 6 ++-- 20 files changed, 83 insertions(+), 83 deletions(-) Patches 1 & 2 applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim v3 1/2] Port to qt5
Hi Jonah, No top posting please :) On 11/7/19 4:26 AM, Jonah Brüchert wrote: Hi Denis, Thank you very much for applying the patches! As mentioned earlier I have worked on a few additional commits to completely replace all deprecated Qt APIs. This is not strictly required until Qt6 is released, but will basically allow to run on Qt6 right when it's released with just a few build system changes. If you are interested, just let me know when you have time to review those as adding those patches is obviously not urgent. Go ahead and send these in. In addition I added a cmake build system which allowed me to work with Qt's QtCreator IDE, but I can very well understand if you don't want to switch the build system and would leave that patch out. I'm not really in charge of the build system. That is Marcel you'd need to convince. He doesn't much like cmake though. I pushed the patches to KDE's GitLab instance https://invent.kde.org/jbbgameich/ofono-phonesim/commits/master for now, but would of course send them to the mailing list for review later. Having them on the list would be good. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim v3 1/2] Port to qt5
Hi Jonah, On 10/30/19 3:37 PM, Jonah Brüchert wrote: From: Simon Busch Co-authored-by: Martin Jansa Co-authored-by: Jonah Brüchert Co-authored-by: Alexander Akulich --- Makefile.am | 4 +++- configure.ac| 15 --- src/control.cpp | 4 ++-- src/phonesim.cpp| 2 +- src/phonesim.h | 2 +- src/qsimcommand.cpp | 2 +- src/qsmsmessage.cpp | 6 +++--- src/server.cpp | 2 +- src/server.h| 2 +- 9 files changed, 25 insertions(+), 14 deletions(-) Sorry for the delay. Took me a bit to sort out Qt5 on my VM. Anyway, the changes look fine to me and things still appear to work. So I went ahead and pushed both of these patches out. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim v2 1/8] Port to qt5
Hi Jonah, On 10/29/19 11:58 AM, Jonah Brüchert wrote: Hi, first of all, thanks for the review! The current structure of the commits is purely for historical reasons, since this is a collection of patches by different people that are all required for the Qt5 port. The Signed-off-by exist for the same reason, because the people who originally did the affected patches used them. Right, but this is sort of irrelevant for phonesim repo itself. I'm fine upgrading to Qt5, but lets structure the commits properly in a logical manner. If you want I can squash them together and group the changes into a commit by content, not by author. That would be preferred. And if you feel obliged to, feel free to make use of Co-Authored-By tags. I'm fine accepting those. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH phonesim v2 1/8] Port to qt5
Hi Jonah, On 10/27/19 11:30 AM, Jonah Brüchert wrote: From: Simon Busch Signed-off-by: Simon Busch Signed-off-by: Martin Jansa We don't use Signed-off-by, so please drop this in the future. --- Makefile.am | 2 +- configure.ac| 6 +++--- src/control.cpp | 4 ++-- src/qsimcommand.cpp | 2 +- src/qsmsmessage.cpp | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) In general we prefer the commits to be separated out between build changes and actual code changes. diff --git a/Makefile.am b/Makefile.am index 2a6fccf..8c99b04 100644 --- a/Makefile.am +++ b/Makefile.am @@ -46,7 +46,7 @@ nodist_src_phonesim_SOURCES = src/ui_controlbase.h \ src_phonesim_LDADD = $(QT_LIBS) -AM_CXXFLAGS = -Wall $(QT_CFLAGS) +AM_CXXFLAGS = -Wall $(QT_CFLAGS) -fPIC -fPIE You set -fPIE here but remove it in patch 6. Do you want to squash these two together? AM_CPPFLAGS = -I$(top_srcdir)/src -I$(top_builddir)/src Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] xmm7modem: fix for crash after receiving MT SMS after SIM swap
Hi Antara, On 10/14/19 11:50 PM, Antara Borwankar wrote: Earlier SMS atom was being created based on +XSIMSTATE command, which is a proprietary command for intel modems. But this logic fails in case of SIM hot swap when +XSIMSTATE value received is not handled in notify function. Which value isn't handled? Should the plugin be handling it instead? There can be other such scenarios as well. Therefore SMS atom will now be created in post_online function which anyways gets called after +XSIMSTATE SMS READY and PB READY is received. Are you sure? Looking at the code it invokes ofono_sim_initialized even after just SIM inserted / PIN not needed. And it is entirely possible for the modem to be Online for emergency calls, in which case SIM insertion or PIN entry will get one to post_online potentially skipping any SMS/phonebook ready notifications. --- plugins/xmm7xxx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] gprs: update attached on netreg updates when running LTE
Hi Richard, On 10/1/19 3:10 PM, richard.rojf...@gmail.com wrote: From: Richard Röjfors There was a race condition where a context might be registered before the netreg status updates to LTE. The code took for granted the context is activated after the technology update. With this change, any order is is accepted. --- src/gprs.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCHv3 2/2] quectel: support own cmux implementation over kernel line discipline
Hi Martin, On 10/7/19 4:39 PM, Martin Hundebøll wrote: The in-kernel implementation of gsm0710 causes deadlocks in the kernel[1], so switch the default back to the user-space implementation in ofono. The change also removes the timeout-callback used to defer disabling the n_gsm line discipline, as that is no longer needed[2] To enable use of the kernel line discipline, add an udev env entry with OFONO_QUECTEL_MUX="n_gsm". [1] https://lore.kernel.org/lkml/4b2455c0-25ba-0187-6df6-c63b4ccc6...@geanix.com/ [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7030082a7415d18e3befdf1f9ec05b3d5de98de4 --- Changes since v2: * keep kernel line discipline support * remove unrelated check in quectel_disable() * remove unrelated setting of AT+IFC=0,0 * revert to using at_util_open_device() plugins/quectel.c | 202 -- plugins/udevng.c | 5 ++ 2 files changed, 166 insertions(+), 41 deletions(-) I split this patch up into two and squashed this warning: plugins/quectel.c:936:14: error: unused variable ‘device’ [-Werror=unused-variable] 936 | GIOChannel *device; | ^~ by taking out the offending variable declaration. Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCHv3 1/2] quectel: rework sim detection
Hi Martin, On 10/7/19 4:39 PM, Martin Hundebøll wrote: Use at_util_sim_state_query_new() to query the sim inserted state. Once that returns, the locked state is queried by issuing a AT+CPIN? command. If not locked, a timer is started to query the quectel init status of the sim. Once the init status is ready, the sim atom is created, and the modem is set to powered, and the sim is signaled both inserted, and initialized. If locked, the modem is set to powered, and the sim atom is created. This allows users to enter the pin to unlock the sim. Once the sim is unlocked, a +CPIN: READY indication is caught to query the quectel init status. Once the init status is ready, the sim is signaled initialized. All the above is needed, because the modem indicated +CPIN: READY before the sim is really ready. The only way to be certain, is to wait for the quectel init status to be ready. Even signaling the sim inserted prematurely can cause to modem to hang during the initial AT+CRSM commands. --- Changes since v2: * None Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCHv3 2/2] gatmux: disable destroy notification on read watcher
Hi Martin, On 10/8/19 1:44 PM, Martin Hundebøll wrote: With the reference in place in received_data(), the address sanitizer now encounters a use-after-free when the destroy notification is dispatched for the read watcher (see below). Fix this by remove the destroy notification callback, as it isn't really used except in the shutdown function. Okay, this is finally coming back to me. So glib does indeed wait until after the source callback has completed to call the destroy function. We have dealt with this in a slightly different way, see GAtIO for details. But I'm fine with this approach as well. Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCHv3 1/2] gatmux: take reference to mux object while processing incoming data
Hi Martin, On 10/8/19 1:44 PM, Martin Hundebøll wrote: When closing down a cmux object, the address sanitizer detects a use-after-free in gatmux.c (see below). Avoid this by taking a reference to the mux object during the processing in received_data(). Applied, thanks. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] gatmux: don't free cmux data until watchers are destroyed
Hi Martin,
I'm afraid this isn't enough, as I still get use-after-free when
using gatmux in the quectel plugin (see attached log).
That buffer size check is a bit dubious and should never happen in
practice, it is also something that one cannot recover from if the
condition does get hit. I actually would just take it out, but you
can also just check the condition right before the mux_unref and
scribble it to a temporary. i.e.
bool buffer_full = false;
g_at_mux_ref();
for (...) {
...
dispatch_sources(...);
}
buffer_full = !mux->shutdown && mux->buf_used == sizeof(mux->buf);
Why the "!mux->shutdown" here? I would expect that to be checked before
processing the data...
Ah you're right, strictly speaking checking mux->shutdown would be
redundant. The idea was to avoid returning FALSE if we're shut down,
but I guess it shouldn't matter.
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCHv2] gatmux: take reference to mux object while processing incoming data
Hi Martin,
@@ -646,13 +653,6 @@ void g_at_mux_unref(GAtMux *mux)
}
}
-static void read_watcher_destroy_notify(gpointer user_data)
-{
- GAtMux *mux = user_data;
-
- mux->read_watch = 0;
-}
-
gboolean g_at_mux_start(GAtMux *mux)
{
if (mux->channel == NULL)
@@ -666,8 +666,7 @@ gboolean g_at_mux_start(GAtMux *mux)
mux->read_watch = g_io_add_watch_full(mux->channel, G_PRIORITY_DEFAULT,
G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
- received_data, mux,
- read_watcher_destroy_notify);
+ received_data, mux, NULL);
mux->shutdown = FALSE;
Can you tell me why this part is needed? Does glib not call
read_watcher_destroy_notify right away when the source is removed?
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] gatmux: don't free cmux data until watchers are destroyed
Hi Martin,
On 10/7/19 1:37 PM, Martin Hundebøll wrote:
Why not do something like:
g_at_mux_ref(mux);
for (i = 1; i <= MAX_CHANNELS && !mux->shutdown; i++) {
int offset = i / 8;
int bit = i % 8;
if (!(mux->newdata[offset] & (1 << bit)))
continue;
dispatch_sources(mux->dlcs[i-1], G_IO_IN);
}
g_at_mux_unref(mux);
I'm afraid this isn't enough, as I still get use-after-free when using
gatmux in the quectel plugin (see attached log).
That buffer size check is a bit dubious and should never happen in
practice, it is also something that one cannot recover from if the
condition does get hit. I actually would just take it out, but you can
also just check the condition right before the mux_unref and scribble it
to a temporary. i.e.
bool buffer_full = false;
g_at_mux_ref();
for (...) {
...
dispatch_sources(...);
}
buffer_full = !mux->shutdown && mux->buf_used == sizeof(mux->buf);
g_at_mux_unref();
...
if (buffer_full)
return FALSE;
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2] ublox: netreg: Also subscribe to UREG URC's
Hi Richard, This is strange. I use gmail smtps, but to check if they tamper with the emails, I cloned a fresh ofono and ran git am and pasted my email, it did work(?). Not sure. I've been using the same process for >10 years now and other patches obtained the same day worked fine. What was strange was that even 'patch -p1' complained of garbage data when I tried to apply the file manually. Line 28 is the first part which looks correct to me. Not really sure why this does not work, I attached it, hope its okay. Anyway, the attachment worked, so I've applied this now. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH v2] ublox: netreg: Also subscribe to UREG URC's
Hi Richard, On 9/30/19 2:04 PM, richard.rojf...@gmail.com wrote: From: Richard Röjfors It turns out that both L2xx and L4xx modems are a bit buggy when it comes to send CREG URC's when the tech changes. Try to overcome this by subscribing to both UREG and CREG, and poll the other when any of the URC's are received. Protect from doing simultaneous polls though. --- drivers/ubloxmodem/network-registration.c | 205 -- 1 file changed, 154 insertions(+), 51 deletions(-) Still no go: denkenz@localhost ~/ofono-master $ git am --3way ~/merge/\[PATCH\ v2\]\ ublox\:\ netreg\:\ Also\ subscribe\ to\ UREG\ URC\'s.eml Applying: ublox: netreg: Also subscribe to UREG URC's Using index info to reconstruct a base tree... error: patch failed: drivers/ubloxmodem/network-registration.c:28 error: drivers/ubloxmodem/network-registration.c: patch does not apply error: Did you hand edit your patch? It does not apply to blobs recorded in its index. Patch failed at 0001 ublox: netreg: Also subscribe to UREG URC's hint: Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] gatmux: don't free cmux data until watchers are destroyed
Hi Martin,
@@ -227,12 +227,15 @@ static void dispatch_sources(GAtMuxChannel *channel,
GIOCondition condition)
static gboolean received_data(GIOChannel *channel, GIOCondition cond,
gpointer data)
{
- GAtMux *mux = data;
+ GAtMux *mux = g_at_mux_ref(data);
So you're taking a ref here unconditionally, yet...
int i;
GIOStatus status;
gsize bytes_read;
if (cond & G_IO_NVAL)
+ goto out_done;
+
+ if (mux->shutdown)
return FALSE;
You don't return it here?
debug(mux, "received data");
@@ -270,15 +273,22 @@ static gboolean received_data(GIOChannel *channel,
GIOCondition cond,
}
if (cond & (G_IO_HUP | G_IO_ERR))
- return FALSE;
+ goto out_done;
if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
- return FALSE;
+ goto out_done;
if (mux->buf_used == sizeof(mux->buf))
- return FALSE;
+ goto out_done;
+
+ g_at_mux_unref(mux);
Why not do something like:
g_at_mux_ref(mux);
for (i = 1; i <= MAX_CHANNELS && !mux->shutdown; i++) {
int offset = i / 8;
int bit = i % 8;
if (!(mux->newdata[offset] & (1 << bit)))
continue;
dispatch_sources(mux->dlcs[i-1], G_IO_IN);
}
g_at_mux_unref(mux);
return TRUE;
+
+out_done:
+ g_at_mux_unref(mux);
+
+ return FALSE;
}
static void write_watcher_destroy_notify(gpointer user_data)
@@ -286,6 +296,9 @@ static void write_watcher_destroy_notify(gpointer user_data)
GAtMux *mux = user_data;
mux->write_watch = 0;
+
+ if (mux->read_watch <= 0 && mux->write_watch <= 0)
+ g_free(mux);
}
static gboolean can_write_data(GIOChannel *chan, GIOCondition cond,
@@ -297,6 +310,9 @@ static gboolean can_write_data(GIOChannel *chan,
GIOCondition cond,
if (cond & (G_IO_NVAL | G_IO_HUP | G_IO_ERR))
return FALSE;
+ if (mux->shutdown)
+ return FALSE;
+
debug(mux, "can write data");
for (dlc = 0; dlc < MAX_CHANNELS; dlc += 1) {
@@ -607,6 +623,7 @@ GAtMux *g_at_mux_new(GIOChannel *channel, const
GAtMuxDriver *driver)
if (mux == NULL)
return NULL;
+
mux->ref_count = 1;
mux->driver = driver;
mux->shutdown = TRUE;
@@ -642,7 +659,8 @@ void g_at_mux_unref(GAtMux *mux)
if (mux->driver->remove)
mux->driver->remove(mux);
- g_free(mux);
+ if (mux->read_watch <= 0 && mux->write_watch <= 0)
+ g_free(mux);
This looks dangerous to me. You're going to cause a crash like this.
You really want to call the disconnect function instead. But I suspect
this is a different patch entirely.
}
}
@@ -651,6 +669,9 @@ static void read_watcher_destroy_notify(gpointer user_data)
GAtMux *mux = user_data;
mux->read_watch = 0;
+
+ if (mux->read_watch <= 0 && mux->write_watch <= 0)
+ g_free(mux);
Same here
}
gboolean g_at_mux_start(GAtMux *mux)
@@ -695,6 +716,7 @@ gboolean g_at_mux_shutdown(GAtMux *mux)
continue;
channel_close((GIOChannel *) mux->dlcs[i], NULL);
+ mux->dlcs[i] = NULL;
This can probably be a separate cleanup patch.
}
if (mux->driver->shutdown)
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH 4/4] quectel: use own cmux implementation over kernel line discipline
Hi Martin, So I wonder whether instead of fully reverting back to CMUX (only to put in n_gsm in later) you might want to just keep both around and use a /etc/quectel.conf or OFONO_QUECTEL_MUX=n_gsm|internal instead? I'd prefer a single method to keep the needed testing to a minimum. I know where you're coming from, but you will probably come back in 6 months and want to use the kernel multiplexer again and this change will get reverted again. Then someone with an old kernel will want the built-in one. Might as well bite the bullet now.. Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] ublox: netreg: Also subscribe to UREG URC's
Hi Richard,
On 9/29/19 2:39 PM, richard.rojf...@gmail.com wrote:
From: Richard Röjfors
It turns out that both L2xx and L4xx modems are a bit
buggy when it comes to send CREG URC's when the tech changes.
Try to overcome this by subscribing to both UREG and CREG,
and poll the other when any of the URC's are received.
Protect from doing simultanious polls though.
nit: typo, simultaneous
Also, this patch doesn't apply?
denkenz@localhost ~/ofono-master $ git am --3way ~/merge/\[PATCH\]\
ublox\:\ netreg\:\ Also\ subscribe\ to\ UREG\ URC\'s.eml
Applying: ublox: netreg: Also subscribe to UREG URC's
Using index info to reconstruct a base tree...
error: patch failed: drivers/ubloxmodem/network-registration.c:47
error: drivers/ubloxmodem/network-registration.c: patch does not apply
error: Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Patch failed at 0001 ublox: netreg: Also subscribe to UREG URC's
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
---
drivers/ubloxmodem/network-registration.c | 198 --
1 file changed, 148 insertions(+), 50 deletions(-)
diff --git a/drivers/ubloxmodem/network-registration.c
b/drivers/ubloxmodem/network-registration.c
index 69af4644..ad905fb3 100644
--- a/drivers/ubloxmodem/network-registration.c
+++ b/drivers/ubloxmodem/network-registration.c
@@ -47,11 +47,15 @@
static const char *none_prefix[] = { NULL };
static const char *cmer_prefix[] = { "+CMER:", NULL };
static const char *ureg_prefix[] = { "+UREG:", NULL };
+static const char *creg_prefix[] = { "+CREG:", NULL };
+
+#define UBLOX_UPDATING_STATUS 0x01
struct netreg_data {
struct at_netreg_data at_data;
const struct ublox_model *model;
+ int flags;
Nowadays we prefer to use bool foo : 1 notation to manipulate bit fields.
So maybe something like:
bool updating_status : 1;
};
struct tech_query {
@@ -213,13 +217,72 @@ static void ctze_notify(GAtResult *result, gpointer
user_data)
ofono_netreg_time_notify(netreg, >time);
}
-static void ublox_query_tech_cb(gboolean ok, GAtResult *result,
+static int ublox_ureg_state_to_tech(int state)
+{
+ switch (state) {
+ case 1:
+ return ACCESS_TECHNOLOGY_GSM;
+ case 2:
+ return ACCESS_TECHNOLOGY_GSM_EGPRS;
+ case 3:
+ return ACCESS_TECHNOLOGY_UTRAN;
+ case 4:
+ return ACCESS_TECHNOLOGY_UTRAN_HSDPA;
+ case 5:
+ return ACCESS_TECHNOLOGY_UTRAN_HSUPA;
+ case 6:
+ return ACCESS_TECHNOLOGY_UTRAN_HSDPA_HSUPA;
+ case 7:
+ return ACCESS_TECHNOLOGY_EUTRAN;
+ case 8:
+ return ACCESS_TECHNOLOGY_GSM;
+ case 9:
+ return ACCESS_TECHNOLOGY_GSM_EGPRS;
+ default:
+ /* Not registered for PS (0) or something unknown (>9)... */
+ return -1;
+ }
+}
+
+static gboolean is_registered(int status)
+{
+ return status == NETWORK_REGISTRATION_STATUS_REGISTERED ||
+ status == NETWORK_REGISTRATION_STATUS_ROAMING;
+}
+
+static void ublox_creg_cb(gboolean ok, GAtResult *result,
+ gpointer user_data)
+{
+ struct tech_query *tq = user_data;
+ struct netreg_data *nd = ofono_netreg_get_data(tq->netreg);
+ int status, lac, ci, tech;
One variable declaration per line please.
+
+ nd->flags &= ~UBLOX_UPDATING_STATUS;
+
+ if (!ok)
+ return;
+
+ if (at_util_parse_reg(result, "+CREG:", NULL, ,
+ , , , OFONO_VENDOR_GENERIC) ==
FALSE)
+ return;
+
+ /* The query provided a tech, use that */
+ if (is_registered(status) && tq->tech != -1)
+ tech = tq->tech;
+
+ ofono_netreg_status_notify(tq->netreg, status, lac, ci, tech);
+}
+
+static void ublox_ureg_cb(gboolean ok, GAtResult *result,
gpointer user_data)
{
struct tech_query *tq = user_data;
+ struct netreg_data *nd = ofono_netreg_get_data(tq->netreg);
GAtResultIter iter;
gint enabled, state;
- int tech = -1;
+ int tech = tq->tech;
+
+ nd->flags &= ~UBLOX_UPDATING_STATUS;
if (!ok)
goto error;
+static void ureg_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_netreg *netreg = user_data;
+ int state;
+ struct netreg_data *nd = ofono_netreg_get_data(netreg);
+ struct tech_query *tq;
+ GAtResultIter iter;
+
+ if (nd->flags & UBLOX_UPDATING_STATUS)
+ return;
+
+ g_at_result_iter_init(, result);
+
+ if (!g_at_result_iter_next(, "+UREG:"))
+
Re: [PATCH] udev: Adding PCIe as a subsystem in udev
Hi Antara,
On 9/27/19 12:51 AM, Antara Borwankar wrote:
Adding support for enumerating PCIe types of modems in ofono
---
plugins/udevng.c | 178 +--
1 file changed, 135 insertions(+), 43 deletions(-)
+static struct {
I made this static const struct {
+ const char *driver;
+ const char *drv;
+ const char *vid;
+ const char *pid;
+} pci_driver_list[] = {
+ { "xmm7xxx", "imc_ipc","0x8086", "0x7560"},
+ { }
+};
+
applied, thanks.
Regards,
-Denis
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
Re: [PATCH] atmodem: CGDCONT handling for cid 0
Hi Antara, On 9/27/19 1:46 AM, Antara Borwankar wrote: Added handling for cid 0 in +CGDCONT callback. --- drivers/atmodem/gprs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c index 16f4927..defbba0 100644 --- a/drivers/atmodem/gprs.c +++ b/drivers/atmodem/gprs.c @@ -240,7 +240,7 @@ static void at_cgdcont_read_cb(gboolean ok, GAtResult *result, return; } - cids = l_uintset_new(activated_cid); + cids = l_uintset_new_from_range(0,activated_cid); Applied with a silent amend to put a space between ',' and 2nd argument. l_uintset_put(cids, activated_cid); Regards, -Denis ___ ofono mailing list -- ofono@ofono.org To unsubscribe send an email to ofono-le...@ofono.org
Re: [RFC] gatmux: don't free cmux data until watchers are destroyed
Hi Martin, My theory is this: * The main loop has (at least) two pending sources: 1) a gatchat callback to e.g. a plugin CFUN=0 command 2) rx data ready on the cmux uart channel * The main loop calls the plugin callback first, where g_at_mux_unref() is called * The main loop then calls the mux->read_watch callback, where the (freed) cmux object is dereferrenced. Hmm, I think it is a little simpler than that...? You have data on CMUX that we process inside dispatch_sources. This in fact calls into GAtIO and thus GAtChat. GAtChat callback results in g_at_mux_unref which destroys the object. But dispatch_sources is still running. An easy fix would be to defer calling g_at_mux_unref until the next event loop iteration (e.g. via l_idle_oneshot). Alternatively, taking a reference to g_at_mux at the beginning of received_data and dropping it at the end might also do the trick. Regards, -Denis ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH] udev: Adding PCIe as a subsystem in udev
Hi Antara,
@@ -1612,16 +1631,8 @@ static void add_device(const char *syspath, const char
*devname,
if (devpath == NULL)
return;
- devnode = udev_device_get_devnode(device);
- if (devnode == NULL) {
- devnode = udev_device_get_property_value(device, "INTERFACE");
- if (devnode == NULL)
- return;
- }
-
- usb_interface = udev_device_get_parent_with_subsystem_devtype(device,
- "usb", "usb_interface");
- if (usb_interface == NULL)
+ subsystem = udev_device_get_subsystem(device);
+ if (subsystem == NULL)
return;
Lets just make the modem type into an argument to this function, since
check_pci_device and check_usb_device already know this.
modem = g_hash_table_lookup(modem_list, syspath);
@@ -1630,7 +1641,12 @@ static void add_device(const char *syspath, const char
*devname,
if (modem == NULL)
return;
- modem->type = MODEM_TYPE_USB;
+ if ((g_str_equal(subsystem, "usb") == TRUE) ||
+ (g_str_equal(subsystem, "usbmisc") == TRUE))
+ modem->type = MODEM_TYPE_USB;
+ else if (g_str_equal(subsystem, "pci") == TRUE)
+ modem->type = MODEM_TYPE_PCIE;
+
modem->syspath = g_strdup(syspath);
modem->devname = g_strdup(devname);
modem->driver = g_strdup(driver);
@@ -1642,30 +1658,60 @@ static void add_device(const char *syspath, const char
*devname,
g_hash_table_replace(modem_list, modem->syspath, modem);
}
- interface = udev_device_get_property_value(usb_interface, "INTERFACE");
- number = udev_device_get_property_value(device, "ID_USB_INTERFACE_NUM");
+ label = udev_device_get_property_value(device, "OFONO_LABEL");
- /* If environment variable is not set, get value from attributes (or parent's ones) */
- if (number == NULL) {
- number = udev_device_get_sysattr_value(device,
+ if (modem->type == MODEM_TYPE_USB) {
+ devnode = udev_device_get_devnode(device);
+ if (devnode == NULL) {
+ devnode = udev_device_get_property_value(device,
+ "INTERFACE");
+ if (devnode == NULL)
+ return;
+ }
+
+ usb_interface = udev_device_get_parent_with_subsystem_devtype(
+ device, "usb",
+ "usb_interface");
+ if (usb_interface == NULL)
+ return;
+
+ interface = udev_device_get_property_value(usb_interface,
+ "INTERFACE");
+ number = udev_device_get_property_value(device,
+ "ID_USB_INTERFACE_NUM");
+
+ if (number == NULL)
+ number = udev_device_get_sysattr_value(device,
"bInterfaceNumber");
- if (number == NULL) {
- parent = udev_device_get_parent(device);
- number = udev_device_get_sysattr_value(parent,
+ if (!label)
+ label = udev_device_get_property_value(usb_interface,
+ "OFONO_LABEL");
+ } else {
+ devnode = NULL;
+ interface = udev_device_get_property_value(device,
+ "INTERFACE");
+ number = udev_device_get_sysattr_value(device,
"bInterfaceNumber");
- }
- }
- label = udev_device_get_property_value(device, "OFONO_LABEL");
- if (!label)
- label = udev_device_get_property_value(usb_interface,
+ if (!label)
+ label = udev_device_get_property_value(device,
"OFONO_LABEL");
+ }
You can drop OFONO_LABEL handling for pci devices completely. I don't
even remember why that was used but it definitely should not be needed
unless you're trying to be backward compatible with some legacy things,
which you're not.
- subsystem = udev_device_get_subsystem(device);
+ /*
+*If environment variable is not set, get value from attributes
+*(or parent's ones)
+*/
+ if (number == NULL) {
+ parent = udev_device_get_parent(device);
+ number = udev_device_get_sysattr_value(parent,
+ "bInterfaceNumber");
+ }
if
Re: [PATCH] udev: Adding PCIe as a subsystem in udev
Hi Antara, On 9/26/19 4:46 AM, Borwankar, Antara wrote: Hi Denis, Regarding your below query - Can you tell me why this new addition uses vid/pid with a 0x prefix while the rest of the table does not? It seems weird & inconsistent. Actually in case of pci device vendor and device id read using libudev API is returning as "0x8086" and "0x7560" strings respectively. So I have added the same values in vendor_list table. That is strange. Okay, so maybe we should just add a different look-up table to handle this. Perhaps pci_driver_list[] and use vids/pids with 0x prefixes there. In case of usb device the vendor and device strings returned by same libudev API does not have "0x" prefix. I don’t have any idea why it is so. If you insist I can remove this prefix 0x from vendor_list table and change the implementation of check_pci_device() function to handle comparison of vendor and model strings with vid, pid form vendor_list. Yep, that might be another alternative. I'd be fine with either. Having some entries with 0x and some without will confuse people and lead to bugs. Regards, -Denis ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
Re: [PATCH 4/4] quectel: use own cmux implementation over kernel line discipline
Hi Martin, On 9/26/19 2:27 PM, Martin Hundebøll wrote: The in-kernel implementation of gsm0710 causes deadlocks in the kernel[1], so switch back to the user-space implementation in ofono. [1] https://lore.kernel.org/lkml/4b2455c0-25ba-0187-6df6-c63b4ccc6...@geanix.com/ --- plugins/quectel.c | 250 -- 1 file changed, 130 insertions(+), 120 deletions(-) Ugh. That poor quectel.c file has so much churn its not funny :) So I wonder whether instead of fully reverting back to CMUX (only to put in n_gsm in later) you might want to just keep both around and use a /etc/quectel.conf or OFONO_QUECTEL_MUX=n_gsm|internal instead? Regards, -Denis ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono
