All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1]
@ 2024-03-28  8:11 Christophe Ronco
  2024-03-28  8:11 ` [PATCH 1/1] atmodem: sim: when reading sim files, avoid incomplete result lines Christophe Ronco
  0 siblings, 1 reply; 6+ messages in thread
From: Christophe Ronco @ 2024-03-28  8:11 UTC (permalink / raw)
  To: ofono; +Cc: Christophe Ronco

Hi,
I use a ME310G1 modem (from Telit). With some SIMs, Ofono is not able to give CardIdentifier property.

This information is normally read in SIM by reading file 12258. 
This is the first AT+CRSM command send to modem.

When not working, it's because in AT command answer, there are two lines
starting with prefix +CRSM. The first line is:
+CRSM: 0
I don't know what does this answer mean, I though that two parameters minimum
(sw1 and sw2) were requested. A correct result line is also sent, but it
won't be analyzed by AT modem driver. It stops at first line with correct prefix.

Here is an extract of traces I have when this behavior is reproduced:
2024-03-26T12:27:22.721536+00:00 klk-zcel-04000C ofonod[1268]: Aux: > AT+CGMI\r
2024-03-26T12:27:22.730407+00:00 klk-zcel-04000C ofonod[1268]: Aux: < \r\nTelit\r\n\r\nOK\r\n
2024-03-26T12:27:22.731175+00:00 klk-zcel-04000C ofonod[1268]: Aux: > AT+CLCK="SC",2\r
2024-03-26T12:27:22.831416+00:00 klk-zcel-04000C ofonod[1268]: Aux: < \r\nOK\r\n
2024-03-26T12:27:22.831805+00:00 klk-zcel-04000C ofonod[1268]: Aux: > AT+CGMM\r
2024-03-26T12:27:22.840647+00:00 klk-zcel-04000C ofonod[1268]: Aux: < \r\nME310G1-WW\r\n\r\nOK\r\n
2024-03-26T12:27:22.842950+00:00 klk-zcel-04000C ofonod[1268]: Aux: > AT+CRSM=192,12258\r
2024-03-26T12:27:23.101668+00:00 klk-zcel-04000C ofonod[1268]: Modem: < \r\n#QSS: 2\r\n
2024-03-26T12:27:23.165293+00:00 klk-zcel-04000C ofonod[1268]: Aux: < \r\n+CRSM: 0\r\n\r\n+CRSM: 144,0,62178202412183022FE28A01058B032F06068002000A880110\r\n\r\nOK\r\n\r\n#QSS: 2\r\n
2024-03-26T12:27:23.166088+00:00 klk-zcel-04000C ofonod[1268]: ../git/plugins/telit.c:qss_notify() 0x1856a00
2024-03-26T12:27:23.166664+00:00 klk-zcel-04000C ofonod[1268]: ../git/plugins/telit.c:switch_sim_state_status() 0x1856a00, SIM status: 2
2024-03-26T12:27:23.167472+00:00 klk-zcel-04000C ofonod[1268]: Aux: > AT+CGMR\r
2024-03-26T12:27:23.176251+00:00 klk-zcel-04000C ofonod[1268]: Aux: < \r\nM0C.200004\r\n\r\nOK\r\n
2024-03-26T12:27:23.178463+00:00 klk-zcel-04000C ofonod[1268]: Aux: > AT+CRSM=192,28421\r
2024-03-26T12:27:23.929127+00:00 klk-zcel-04000C ofonod[1268]: Aux: < \r\n+CRSM: 144,0,62178202412183026F058A01058B036F060E80020008880110\r\n\r\nOK\r\n

The aim of this patch is to ignore result lines that do not contain at least sw1 and sw2 parameters.

Christophe Ronco (1):
  atmodem: sim: when reading sim files, avoid incomplete result lines

 drivers/atmodem/sim.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/1] atmodem: sim: when reading sim files, avoid incomplete result lines
  2024-03-28  8:11 [PATCH 0/1] Christophe Ronco
@ 2024-03-28  8:11 ` Christophe Ronco
  2024-03-28 15:17   ` Denis Kenzior
  0 siblings, 1 reply; 6+ messages in thread
From: Christophe Ronco @ 2024-03-28  8:11 UTC (permalink / raw)
  To: ofono; +Cc: Christophe Ronco

Modem ME310G1 sometimes add incomplete result lines to first AT+CRSM
command. Example:
AT+CRSM=192,12258
+CRSM: 0
+CRSM: 144,0,62178202412183022FE28A01058B032F06068002000A880110

OK

Parse all result lines starting with prefix until a line with at least sw1
and sw2 parameters is found.
---
 drivers/atmodem/sim.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index d75a09c2..46f21758 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -107,6 +107,7 @@ static void get_response_common_cb(gboolean ok, GAtResult *result,
 	int str;
 	unsigned char access[3];
 	unsigned char file_status;
+	gboolean nb_found = FALSE;
 
 	decode_at_error(&error, g_at_result_final_response(result));
 
@@ -117,11 +118,14 @@ static void get_response_common_cb(gboolean ok, GAtResult *result,
 
 	g_at_result_iter_init(&iter, result);
 
-	if (!g_at_result_iter_next(&iter, prefix))
-		goto error;
+	while (!nb_found) {
+		if (!g_at_result_iter_next(&iter, prefix))
+			goto error;
 
-	g_at_result_iter_next_number(&iter, &sw1);
-	g_at_result_iter_next_number(&iter, &sw2);
+		nb_found = g_at_result_iter_next_number(&iter, &sw1);
+		if (nb_found)
+			nb_found = g_at_result_iter_next_number(&iter, &sw2);
+	}
 
 	if (!g_at_result_iter_next_hexstring(&iter, &response, &len) ||
 			(sw1 != 0x90 && sw1 != 0x91 && sw1 != 0x92) ||
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/1] atmodem: sim: when reading sim files, avoid incomplete result lines
  2024-03-28  8:11 ` [PATCH 1/1] atmodem: sim: when reading sim files, avoid incomplete result lines Christophe Ronco
@ 2024-03-28 15:17   ` Denis Kenzior
  2024-05-07 14:46     ` [PATCH v2 0/1] " Christophe Ronco
  0 siblings, 1 reply; 6+ messages in thread
From: Denis Kenzior @ 2024-03-28 15:17 UTC (permalink / raw)
  To: Christophe Ronco, ofono

Hi Christophe,

On 3/28/24 03:11, Christophe Ronco wrote:
> Modem ME310G1 sometimes add incomplete result lines to first AT+CRSM
> command. Example:
> AT+CRSM=192,12258
> +CRSM: 0
> +CRSM: 144,0,62178202412183022FE28A01058B032F06068002000A880110
> 
> OK
> 
> Parse all result lines starting with prefix until a line with at least sw1
> and sw2 parameters is found.
> ---
>   drivers/atmodem/sim.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 

<snip>

> @@ -117,11 +118,14 @@ static void get_response_common_cb(gboolean ok, GAtResult *result,
>   
>   	g_at_result_iter_init(&iter, result);
>   
> -	if (!g_at_result_iter_next(&iter, prefix))
> -		goto error;
> +	while (!nb_found) {
> +		if (!g_at_result_iter_next(&iter, prefix))
> +			goto error;
>   
> -	g_at_result_iter_next_number(&iter, &sw1);
> -	g_at_result_iter_next_number(&iter, &sw2);
> +		nb_found = g_at_result_iter_next_number(&iter, &sw1);
> +		if (nb_found)
> +			nb_found = g_at_result_iter_next_number(&iter, &sw2);
> +	}

I think you're going to need a comment in the code, there's no way anyone will 
understand what is happening here after a year or two.

Also consider a few other alternatives:

if (!g_at_result_iter_next(&iter, prefix))
	goto error;

/* Skip to last response, include comment as to why */
while (g_at_result_iter_next(&iter, prefix))
	;

... proceed as normal

Alternatively
const guint8 *response = NULL;

...

while (g_at_result_iter_next(&iter, prefix)) {
	if (!g_at_result_iter_next_number(&iter, &sw1))
		continue;

	/* validate sw1, can be 0x9* or 0x6* */
	if (!valid_sw1(...))
		continue;

	if (!g_at_result_iter_next(&iter, &sw2))
		continue;

	if (!g_at_result_iter_next_hexstring....) {
		cb(...)
		return;
	}
}

if (!response)
	goto error;

/* parse response */

>   
>   	if (!g_at_result_iter_next_hexstring(&iter, &response, &len) ||
>   			(sw1 != 0x90 && sw1 != 0x91 && sw1 != 0x92) ||

Regards,
-Denis

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 0/1] atmodem: sim: when reading sim files, avoid incomplete result lines
  2024-03-28 15:17   ` Denis Kenzior
@ 2024-05-07 14:46     ` Christophe Ronco
  2024-05-07 14:46       ` [PATCH 1/1] " Christophe Ronco
  2024-05-07 16:40       ` [PATCH v2 0/1] " patchwork-bot+ofono
  0 siblings, 2 replies; 6+ messages in thread
From: Christophe Ronco @ 2024-05-07 14:46 UTC (permalink / raw)
  To: ofono; +Cc: Christophe Ronco

Hi Denis,

Here is the second version of the patch.

Sorry for the delay,

Christophe

Christophe Ronco (1):
  atmodem: sim: when reading sim files, avoid incomplete result lines

 drivers/atmodem/sim.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/1] atmodem: sim: when reading sim files, avoid incomplete result lines
  2024-05-07 14:46     ` [PATCH v2 0/1] " Christophe Ronco
@ 2024-05-07 14:46       ` Christophe Ronco
  2024-05-07 16:40       ` [PATCH v2 0/1] " patchwork-bot+ofono
  1 sibling, 0 replies; 6+ messages in thread
From: Christophe Ronco @ 2024-05-07 14:46 UTC (permalink / raw)
  To: ofono; +Cc: Christophe Ronco

Modem ME310G1 sometimes add incomplete result lines to first AT+CRSM
command. Example:
AT+CRSM=192,12258
+CRSM: 0
+CRSM: 144,0,62178202412183022FE28A01058B032F06068002000A880110

OK

Parse all result lines starting with prefix until a line with at least sw1
and sw2 parameters is found.
---
 drivers/atmodem/sim.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index d75a09c2..1f63090b 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -107,6 +107,7 @@ static void get_response_common_cb(gboolean ok, GAtResult *result,
 	int str;
 	unsigned char access[3];
 	unsigned char file_status;
+	gboolean result_found = FALSE;
 
 	decode_at_error(&error, g_at_result_final_response(result));
 
@@ -117,11 +118,18 @@ static void get_response_common_cb(gboolean ok, GAtResult *result,
 
 	g_at_result_iter_init(&iter, result);
 
-	if (!g_at_result_iter_next(&iter, prefix))
-		goto error;
+	/* some Telit modems sometimes return incomplete result lines */
+	/* get first result line with mandatory parameters */
+	while (g_at_result_iter_next(&iter, prefix)) {
+		if (!g_at_result_iter_next_number(&iter, &sw1))
+			continue;
+		if (!g_at_result_iter_next_number(&iter, &sw2))
+			continue;
+		result_found = TRUE;
+	}
 
-	g_at_result_iter_next_number(&iter, &sw1);
-	g_at_result_iter_next_number(&iter, &sw2);
+	if (!result_found)
+		goto error;
 
 	if (!g_at_result_iter_next_hexstring(&iter, &response, &len) ||
 			(sw1 != 0x90 && sw1 != 0x91 && sw1 != 0x92) ||
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/1] atmodem: sim: when reading sim files, avoid incomplete result lines
  2024-05-07 14:46     ` [PATCH v2 0/1] " Christophe Ronco
  2024-05-07 14:46       ` [PATCH 1/1] " Christophe Ronco
@ 2024-05-07 16:40       ` patchwork-bot+ofono
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+ofono @ 2024-05-07 16:40 UTC (permalink / raw)
  To: Christophe Ronco; +Cc: ofono

Hello:

This patch was applied to ofono.git (master)
by Denis Kenzior <denkenz@gmail.com>:

On Tue,  7 May 2024 16:46:17 +0200 you wrote:
> Hi Denis,
> 
> Here is the second version of the patch.
> 
> Sorry for the delay,
> 
> Christophe
> 
> [...]

Here is the summary with links:
  - [1/1] atmodem: sim: when reading sim files, avoid incomplete result lines
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=7802d80a9619

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-05-07 16:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-28  8:11 [PATCH 0/1] Christophe Ronco
2024-03-28  8:11 ` [PATCH 1/1] atmodem: sim: when reading sim files, avoid incomplete result lines Christophe Ronco
2024-03-28 15:17   ` Denis Kenzior
2024-05-07 14:46     ` [PATCH v2 0/1] " Christophe Ronco
2024-05-07 14:46       ` [PATCH 1/1] " Christophe Ronco
2024-05-07 16:40       ` [PATCH v2 0/1] " patchwork-bot+ofono

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.