qmi-codegen in libqmi-glib

[Aleksander Morgado]

On 05/23/2012 10:53 PM, Dan Williams wrote:
> On Wed, 2012-05-23 at 22:21 +0200, Aleksander Morgado wrote:
>>>>
>>>> I've been playing a bit more with the idea of automatically generating
>>>> code for message handling from input JSON files, and got already
>>>> something working in the 'qmi-codegen' branch of my libqmi-glib tree:
>>>>   git://gitorious.org/lanedo/libqmi-glib.git
>>>>
>>>> The current set of supported messages are defined in the following JSON
>>>> files:
>>>>
>>>> https://gitorious.org/lanedo/libqmi-glib/blobs/qmi-codegen/data/qmi-common.json
>>>>
>>>> https://gitorious.org/lanedo/libqmi-glib/blobs/qmi-codegen/data/qmi-service-ctl.json
>>>>
>>>> https://gitorious.org/lanedo/libqmi-glib/blobs/qmi-codegen/data/qmi-service-dms.json
>>>>
>>>> https://gitorious.org/lanedo/libqmi-glib/blobs/qmi-codegen/data/qmi-service-wds.json
>>>>
>>>> And adding new messages should need, ideally, just updating these JSON
>>>> files. I guess the qmi-codegen will need updates from time to time when
>>>> new basic types are found in the QMI messages. Currently the following
>>>> types are supported:
>>>>  * integers (uint8/uint16/uint32/int8/int16/int32)
>>>>  * strings
>>>>  * structs with integer contents
>>>>  * arrays of structs
>>>
>>> At some point we'll need unions too, where the member of the union
>>> that's actually used depends on a specific value from earlier in the
>>> struct.  That's how stuff like "get the access technology and status"
>>> works where you've got a big struct like:
>>>
>>> struct {
>>>     bool 3gpp;  // true = 3gpp, false = 3gpp2
>>>     union {
>>>         struct {
>>>             uint mcc;
>>>             uint mnc;
>>>             uint lac;
>>>             uint cid;
>>>             enum rat_type;
>>>         } 3gpp;
>>>         struct {
>>>             bool is_1x_or_hdr;
>>>             union {
>>>                 struct {
>>>                     1x junk;
>>>                 } 1x;
>>>                 struct {
>>>                     evdo junk;
>>>                 } hdr;
>>>             } u2;
>>>         } 3gpp2;
>>>     } u;
>>> }
>>>
>>> that's some evil stuff, but basically  normal protocol packet parsing :(
>>> Anyway, I do like the idea of doing this with JSON.  I despair of
>>> actually being able to use the Gobi database except for reference since
>>> generating the code out of them is horrendously complicated.
>>
>>
>> I'll check that message during the next days and also try to provide
>> codegen for it.
>>
>> Still have other fixes in mind for it, but at least as it is now it
>> should be equivalent feature-wise to the master branch.
> 
> eg sWDSEventReportIndication_DataSystems in
> GobiConnectionMgmtAPIStructs.h.  That's basically a worst-case scenario,
> but unfortunately one that is kinda important.  The bitfields and
> padding there just make it worse, though ideally we can reduce the
> bitfields to flag enums or something.  That struct isn't quite as bad as
> it looks, since in reality all of sNetworkTypeIs1 can be represented by:
> 
> enum CdmaType {
>     /* 1x, evdo0, evdoA, and evdoB are mutually exclusive,
>      * but eHRPD could be set when any of evdo0, evdoA,
>      * or evdoB are set.
>      */
>     1x = 0x01;
>     evdo0 = 0x02;
>     evdoA = 0x04;
>     evdoB = 0x08;
>     eHRPD = 0x10;
> };
> 
> enum Cdma1xInfo {
>     Cdma1x_is95 = 0x01;
>     Cdma1x_is2000 = 0x02;
>     Cdma1x_is2000RelA = 0x04;
> };
> 
> enum Evdo0Info {
>     Evdo0_dpa = 0x01;
> };
> 
> enum EvdoAInfo {
>     EvdoA_dpa = 0x01;
>     EvdoA_mfpa = 0x02;
>     EvdoA_empa = 0x04;
>     EvdoA_empa_ehrpd = 0x08;
> };
> 
> enum EvdoBInfo {
>     EvdoB_dpa = 0x01;
>     EvdoB_mfpa = 0x02;
>     EvdoB_empa = 0x04;
>     EvdoB_empa_ehrpd = 0x08;
>     EvdoB_mmpa_ehrpd = 0x10;
>     EvdoB_mmpa_ehrpd = 0x20;
> };
> 
> struct sNetworkTypeIs1 {
>     enum:uint8 CdmaType cdma_type;
>     uint8 padding1[2];
>     uint8 null_bearer; // 0x80 = TRUE, 0x0 = FALSE
>     union {
>         enum:uint8 Cdma1xInfo 1xinfo;
>         enum:uint8 Evdo0Info  e0info;
>         enum:uint8 EvdoAInfo  eAinfo;
>         enum:uint8 EvdoBInfo  eBinfo;
>     };
>     uint8 padding2[3];
> }; 
> 

I already handled something like that in the current code, the result
for the "Get Current Data Bearer Technology". Instead of the real enums
being specified in the last union, I just used a generic guint32
'so_mask'. Then, each enum/flags defined has proper documentation on
where to use it, like:

/**
 * QmiWdsSoCdma1x:
 * @QMI_WDS_SO_CDMA1X_NONE: Unknown, to be ignored.
 * @QMI_WDS_SO_CDMA1X_IS95: IS95.
 * @QMI_WDS_SO_CDMA1X_IS2000: IS2000.
 * @QMI_WDS_SO_CDMA1X_IS2000_REL_A: IS2000 RelA.
 *
 * Flags specifying the Service Option when the bearer network type is
 * @QMI_WDS_NETWORK_TYPE_3GPP2 and when the Radio Access Technology mask
 * contains @QMI_WDS_RAT_3GPP2_CDMA1X.
 */
typedef enum {
    QMI_WDS_SO_CDMA1X_NONE         = 0,
    QMI_WDS_SO_CDMA1X_IS95         = 1 << 0,
    QMI_WDS_SO_CDMA1X_IS2000       = 1 << 1,
    QMI_WDS_SO_CDMA1X_IS2000_REL_A = 1 << 2
} QmiWdsSoCdma1x;

The user of the library *needs* to know when the so_mask contains a
specific set of flags. The user needs to know it both in this case,
where you get a generic mask, in order to know which set of flags to
use; but it also needs to know it if we provide a union of different
fields, in order to know which field of the union is the appropriate
one. Aren't then the unions (at least in this case) just an additional
complication that we can just completely skip?

-- 
Aleksander