Hey,
> > > I am trying to update the code for the Radio/Bands of Cinterion
> > > modules, and it seems a nasty job, because of the way the indicators
> > > evolved.
> > > 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")
> > > ...
> > >
> > > where all the bands for 2G+3G fit in an unsigned integer on 32 bits,
> > > ie, the value 511 above.
> > > The format is decimal, but the interpretation is a bitmap.
> > > This is done well in the current code and works well.
> > > One possible flaw is that the detection of the charset is not perfect,
> > > but the above output is invariant also when UCS2 is used.
> > >
> > > When we go to LTE, however, the 32 bits are not enough to host all
> > > bands, and so it was decided to split the command by technologies.
> > > In these implementations there are some divergences.
> > > For instance, 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")
> > > ...
> > >
> > > While the PLPS9 reports (invariant under charset change):
> > > ...
> > > ^SCFG: "Radio/Band/2G",("00000001-0000000f"),,("0","1")
> > > ^SCFG: "Radio/Band/3G",("00000001-000400b5"),,("0","1")
> > > ^SCFG:
> > > "Radio/Band/4G",("00000001-8a0e00d5"),("00000002-000001e2"),("0","1")
> > >
> > > note also the second range for the LTE bands >32
> > > ...
> > >
> > > My idea is to parse first the old format, then the newer one. I have a
> > > regular expression that works well for all 3 formats for the
> > > Radio/Band/xG syntax.
> > > Q1: Do you think this strategy is ok?
> > >
> >
> > It probably is, yes. The fact that the modem manufacturers don't
> > usually keep the same AT command reference for all their devices is
> > extremely annoying as you just found out...
>
> Oh well, for this one the fault is the lack of crystal ball :)
> I don't think they imagined once they launched 3G that they needed
> more than 32 bands, and the prediction even hold for quite some time
> for LTE.
That's not an excuse to break backwards compatibility of the AT
command set. It's fine to extend the AT command/responses, but
*changing* the format for every new model iteration, making the format
incompatible with older devices, that's just bad practice IMO. I guess
they just didn't care, because each new device has its own Windows
driver, and that's probably the only thing they cared for.
> About the inconsistency of point 3 below, this is disappointing
> indeed, but it happens sometime when there are more groups working on
> different projects in parallel.
> And once a product or a feature is on the field, it is quite
> impossible to come back. This would put off the current users of that
> feature that have perhaps adapted their applications.
>
> >
> > > 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.
>
> the plugins/cinterion/mm-broadband-modem-cinterion.c call is:
> !mm_cinterion_parse_scfg_test (response,
> g_task_get_source_object (task),
> mm_base_modem_get_product_id (_self),
>
> mm_broadband_modem_get_current_charset (MM_BROADBAND_MODEM (self)),
> &bands,
> &error))
>
> and so mm_broadband_modem_get_current_charset returns
> MM_MODEM_CHARSET_UNKNOWN.
> I am not really surprised because I see no call to AT+CSCS? on the code.
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?
>
That should already be happening. I wonder if it's disabled in
Cinterion plugin for some reason I don't recall.
> In any case, I do need to check whether I have "0x" at the beginning
> of the string (because not all formats have it), and so checking also
> for 00300078 (the corresponding UCS2 coding) is not a big issue.
>
That's true; but especially on number parsing, you cannot truly say
whether a number like "00300078" is in hex or dec just looking at that
string. If it has a 0x prefix, yes, in hex, but otherwise, it's not
clear enough I believe.
> >
> > > The charset transformation with mm_charset_take_and_convert_to_utf8()
> > > seems to work well only with MM_MODEM_CHARSET_UCS2, which is ok
> > > because it is the only conversion needed.
> > > Q2: Is there a better way?
> > >
> >
> > If the modem is configured for UCS2,
> > mm_broadband_modem_get_current_charset() should return
> > MM_MODEM_CHARSET_UCS2. Would not that be enough? Is that not the case?
>
> that would indeed be enough, and no, it is not the case.
>
We do need to understand why that happens. Charset setup should happen
during init.
> >
> > > And finally, the main question: please notice above that the PLS62
> > > range (and modules in this family) for GSM is different than from any
> > > other module:
> > > 0x00000004, MM_MODEM_BAND_EGSM
> > > 0x00000010, MM_MODEM_BAND_DCS
> > > 0x00000020, MM_MODEM_BAND_PCS
> > > 0x00000040, MM_MODEM_BAND_G850
> > >
> > > I am thinking about passing the USB pid to the fuction, since I can't
> > > find an internal way to discriminate here (for the
> > > mm_cinterion_parse_scfg_test() could be possible, but not for the
> > > mm_cinterion_parse_scfg_response(): in case the current GSM band is
> > > '4', would be 900MHz for the PLS62, and 850 for all others).
> > > Is there another way?
> >
> > You could use the already available
> > mm_kernel_device_get_physdev_pid(), but for this specific case I would
> > probably use a different way to handle it. Instead of hardcoding the
> > PID check in the code, you could use a udev tag to tag the PLS62 and
> > then use that in the code. See the usage of ID_MM_TELIT_BND_ALTERNATE
> > in the code for a very similar case in telit modems.
>
> looks perfect, I will do this way.
> I have anyway a newer udev rule for the PLS62 enumeration (005b), that
> cuts short the check for AT ports.
That's nice, please push that in a separate MR :D Thanks!
--
Aleksander
https://aleksander.es
_______________________________________________
ModemManager-devel mailing list
ModemManager-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/modemmanager-devel
