Attention is currently required from: lynxis lazus, pespin.

Patch set 6:Code-Review -1

View Change

4 comments:

• File sgsn/BSSGP_ConnHdlr.ttcn:

  • Patch Set #6, Line 240: as_mm_identity

    Maybe deriving two smaller altsteps (`as_mm_identity_imsi` and `as_mm_identity_imei`) from this one would be more flexible? The existing `as_mm_identity` would just combine both like this:

    ```

    altstep as_mm_identity(integer ran_index := 0) runs on BSSGP_ConnHdlr {
        [] as_mm_identity_imsi(ran_index);
        [] as_mm_identity_imei(ran_index);
    }
    ```

    This would allow the API user to expect (or not expect) specific identity, e.g. you could use `as_mm_identity_imei` below to expect the IMEI request specifically.

  • Patch Set #6, Line 265: as_receive_l3

    This again looks 99% identical to the existing `f_receive_l3()`, so again code duplication. An altstep is indeed more flexible, but having more than one API doing the same thing is not good. I suggest removing `f_receive_l3()` and changing the code to use your new altstep.

  • Patch Set #6, Line 578: allow_id_imei_req

    We may still receive an identity request here (regardless of the expectations), and this would block the altstep until `Tguard` fires... With the approach of two smaller altsteps I suggested above, you could do something like this:

    ```
    [allow_id_imei_req] as_mm_identity_imei(ran_index) { repeat; }
    [] as_mm_identity(ran_index) {
        setverdict(fail, "Rx unexpected MM IDENTITY Req");
        }
    [] as_receive_l3(tr_GMM_ATTACH_ACCEPT('001'B, ?, ?), l3_mt, ran_index) { ... }
    }
    ```

  • Patch Set #6, Line 579: f_process_attach_accept

    (cosmetic, but) please move the function call to its own line.

To view, visit change 38398. To unsubscribe, or for help writing mail filters, visit settings.