Re: [PATCH 6/9] gatchat: fix gatsyntax to support +CPOS

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2006 bytes --]

Hi Jarko,

On 03/08/2011 05:00 AM, Jarko Poutiainen wrote:
> ---
>  gatchat/gatsyntax.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/gatchat/gatsyntax.c b/gatchat/gatsyntax.c
> index 2fc70b8..a85bc66 100644
> --- a/gatchat/gatsyntax.c
> +++ b/gatchat/gatsyntax.c
> @@ -90,7 +90,11 @@ static GAtSyntaxResult gsmv1_feed(GAtSyntax *syntax,
>  
>  		case GSMV1_STATE_INITIAL_CR:
>  			if (byte == '\n')
> -				syntax->state = GSMV1_STATE_INITIAL_LF;
> +				if (*len == 2) {

You simply cannot do this.  The whole idea behind the state machine in
GAtSyntax is that your data is a byte-stream.  There are no markers, and
you can make no assumptions on the length of the buffer that is fed into
this function.  e.g. if you're unlucky you can get:

gsmv1_feed("\r")
gsmv1_feed("\n")

> +					res = G_AT_SYNTAX_RESULT_PROMPT;
> +					syntax->state = GSMV1_STATE_IDLE;
> +				} else
> +					syntax->state = GSMV1_STATE_INITIAL_LF;

Is there any particular reason why CPOS is not re-using the prompting
feature of AT commands and has to invent its own?  If you think about
this a bit harder you will realize that the spec is *completely* broken
here and introduces a parser ambiguity which cannot be reliably fixed.

At this point my only suggestion is that you need to set a parser hint
after every CPOS command sent to the modem.  Or better yet, push a
change request to 27.007 to use proper prompting procedures.

>  			else
>  				syntax->state = GSMV1_STATE_ECHO;
>  			break;
> @@ -252,7 +256,9 @@ static GAtSyntaxResult gsm_permissive_feed(GAtSyntax *syntax,
>  
>  		switch (syntax->state) {
>  		case GSM_PERMISSIVE_STATE_IDLE:
> -			if (byte == '\r' || byte == '\n')
> +			if (byte == '\n' && *len == 2)
> +				res = G_AT_SYNTAX_RESULT_PROMPT;
> +			else if (byte == '\r' || byte == '\n')
>  				/* ignore */;
>  			else if (byte == '>')
>  				syntax->state = GSM_PERMISSIVE_STATE_PROMPT;

Regards,
-Denis