Hey Aleksander, all,
I have finally submitted a merge request for the band reading and
setting for Cinterion Modems.
Not sure whether is it good quality, thou. I would appreciate a review
and comments.
Starting for example from the naming of the udev variable, and then
the use of other variables to track formats, and the existing 2g/3g
one, also used for differentiating models.
I am wondering if there is a smart way to reduce all these variables
and corresponding if()'s to something more structured.
Apart from that, the code works.
The Band setting also goes well, but...
1) the command is veeeerry slow (about 30-50 seconds) on PLS62, but
there seems to be a problem with MM itself, because I have set a
timeout to 3 minutes, and sometimes MM still times out, but the
command has already succeeded and the bands are changed.
Any idea why the command would timeout? is there a maximum timeout
that MM would tolerate? How to go round this limitation?
It looks like that the PLS62 applies the bands at once, rescans
(so also it de-registers and re-registers), and I think this is the
reason why it is slow.
2) If I reduce the current mask, say from
[1,2,3,4,5,7,9,10,11,12,31,32,33,34,35,37,38,42,48,49,50,58,219]
to [1,2,10,11,12,48,49,50,58], and I re-read the CurrentBands
property, a realloc fails and MM crashes.
It seems to be completely on the dbus handling, and also
independent from this patch.
I have traced it with gdb, but wasn't able to get to the root of
the issue, because the stack starts from the dbus get property call.
If I don't read the property, it works fine.
Also, if the setting is done in the other direction (from less to
more bands), then it works.
Any idea?
Thank you,
Regards,
Giacinto
On Mon, May 18, 2020 at 4:55 PM Aleksander Morgado
<aleksan...@aleksander.es> wrote:
>
> Hey,
>
> > > > > > > > > > I am trying to update the code for the Radio/Bands of
> > > > > > > > > > Cinterion
> > > > > > > > > > Today in the code there is a parser for this format (only
> > > > > > > > > > for the test part):
> > > > > > > > > > AT^SCFG=?
> > > > > > > > > > ^SCFG: "Radio/Band",("1-479","0-1")
> > > > > > > > > >
> > > > > > > > > > the PLS62 reports:
> > > > > > > > > > under GSM charset:
> > > > > > > > > > AT^SCFG=?
> > > > > > > > > > ^SCFG: "Radio/Band/2G",("0x00000004"-"0x00000074")
> > > > > > > > > > ^SCFG: "Radio/Band/3G",("0x00000001"-"0x0004019B")
> > > > > > > > > > ^SCFG: "Radio/Band/4G",("0x00000001"-"0x080E08DF")
> > > > > > > > > > and under UCS2 charset:
> > > > > > > > > > AT^SCFG=?
> > > > > > > > > > ^SCFG:
> > > > > > > > > > "Radio/Band/2G",("0030007800300030003000300030003000300034"-"0030007800300030003000300030003000370034")
> > > > > > > > > > ^SCFG:
> > > > > > > > > > "Radio/Band/3G",("0030007800300030003000300030003000300031"-"0030007800300030003000340030003100390042")
> > > > > > > > > > ^SCFG:
> > > > > > > > > > "Radio/Band/4G",("0030007800300030003000300030003000300031"-"0030007800300038003000450030003800440046")
> > > > > > > > >
> > > > > > > > > > To determine the charset, I use some heuristics on string
> > > > > > > > > > length and
> > > > > > > > > > prefix (0x aka 00300078). Note also that a model returns
> > > > > > > > > > lowercase
> > > > > > > > > > hexa while another uppercase...
> > > > > > > > >
> > > > > > > > > Isn't mm_broadband_modem_get_current_charset () not returning
> > > > > > > > > the
> > > > > > > > > correct thing? I would definitely avoid doing heuristics to
> > > > > > > > > detect
> > > > > > > > > charset if possible.
> > > > > > > >
> > > > > > > Is there not a CSCS=? check during initialization, plus a CSCS=X
> > > > > > > set
> > > > > > > command after that? I believe that ModemManager tries to set the
> > > > > > > "best" charset of the ones available. Do you have a full debug
> > > > > > > log to
> > > > > > > check how that is happening?
> > > > > > >
> > > > > > > > The only time it is called is after setting it, and it looks
> > > > > > > > like it
> > > > > > > > requires a dbus input, and therefore can only happen after the
> > > > > > > > modem
> > > > > > > > is detected.
> > > > > > >
> > > > > > > No no, there is no charset setting based on DBus calls. It's done
> > > > > > > during init.
> > > > > > >
> > > > > > > > Unfortunately the call above happens during the modem detection
> > > > > > > > phase.
> > > > > > > >
> > > > > > > > one fix could be sending the line
> > > > > > > > AT+CSCS="GSM"
> > > > > > > > in the initialization, along with other test commands like
> > > > > > > > AT*CNTI=2?
> > > > > > > >
> > > > > >
> > > > > > I have checked the code, and the charset detection&set is part of
> > > > > > the
> > > > > > enable state machine, while the radio/band detection is part of the
> > > > > > modem discovery state machine in :src/mm-iface-modem.c:
> > > > > >
> > > > > > typedef enum {
> > > > > > ...
> > > > > > ENABLING_STEP_SUPPORTED_CHARSETS,
> > > > > > ENABLING_STEP_CHARSET,
> > > > > > ...
> > > > > > } EnablingStep;
> > > > > >
> > > > > > typedef enum {
> > > > > > ...
> > > > > > INITIALIZATION_STEP_SUPPORTED_BANDS,
> > > > > > ...
> > > > > > INITIALIZATION_STEP_CURRENT_BANDS,
> > > > > > ...
> > > > > > } InitializationStep;
> > > > > >
> > > > > > It is possible that other initialization steps require the right
> > > > > > charset. Wouldn't be good to move the charset settings among these
> > > > > > steps?
> > > > > > After all setting the charset is part of setting up the
> > > > > > communication
> > > > > > channel, a bit like the baudrate and similar parameters.
> > > > > >
> > > > >
> > > > > That makes sense I think, yes. Would you like to send a patch for
> > > > > that?
> > > > >
> > > >
> > > > Yes, however I fear some regressions. For example, things work in the
> > > > current Cinterion supported/current bands because the charset is
> > > > 'unknown'.
> > > > I would propose to split the supported & detection during the initial
> > > > phase, and the actual setting of the charset where it is.
> > > >
> > >
> > > I'm not really sure about that; you actually convinced me when you
> > > said that initializing charset is a bit like initializing the serial
> > > channel :D
> > > Why do you say that things work because the channel is unknown?
> > >
> >
> > I have moved the two procedures (need to clean the code before
> > submitting) to the init phase, and seems to work well, so I will
> > submit the change.
> >
> > My point above is the following. During the init phase, all
> > conversions are handled as follow:
> > if (charset != MM_MODEM_CHARSET_UNKNOWN)
> > maxbandstr = mm_charset_take_and_convert_to_utf8
> > (maxbandstr, charset);
> >
> > since the charset was not initialized so far, we are guaranteed that
> > the if() above fails, and therefore whatever follows has been tested
> > on untranslated strings.
> > What if now the string is different?
> > I can and will test for the modems I have, but cannot guarantee for
> > other plugins.
> >
>
> I believe this is a good risk to take; at the end, converting from a
> known charset should be better than the opposite (trying to guess
> charset I assume).
> We can put this in git master, and I'll test with the devices I have with me.
>
> Also, I'm thinking in preparing a pre-release version for 1.14, so
> this change would be a good candidate to get in that pre-release
> version.
>
> --
> Aleksander
> https://aleksander.es
_______________________________________________
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
