Attention is currently required from: fixeria, dexter.

View Change

8 comments:

• File doc/timeslot.msc:

  • Patch Set #2, Line 95: bsc_ts abox bsc_ts [label="UNUSED"];

    Thats true, Can we shorten this since the mechanism is already explained above. […]

    (Dots are not sufficient to indicate an omission, they mean to wait for an async event.)

  • Patch Set #2, Line 98: "

    Done

    (I actually meant literally "\n" to force a line break, but i tested and it looks fine like this)

    https://people.osmocom.org/neels/kes0Chie/timeslot.png

• File doc/timeslot.msc:

  • Patch Set #9, Line 95:

    bsc_ts box bsc_ts [label="release PDCH"];
    	...;
    

    ok i'll be happy if you replace these two lines with

       bts note bsc_ts [label="PDCH release, see WAIT_PDCH_DEACT above"]

    (i.e. use a "note" and explicitly refer to another place, and span across all the lines involved)

  • Patch Set #9, Line 104:

    	bsc_ts box bsc_ts [label="activate PDCH"];
    	...;
    

    ...and replace these two lines with

       bts note bsc_ts [label="PDCH activation, see WAIT_PDCH_ACT above"]

• File src/osmo-bsc/pcu_sock.c:

  • Patch Set #9, Line 803: * not need any extra activation/deactivation of PDCHs. */

    i thought more like this:

     /* BSC co-located PCU applies only to Ericsson RBS, which supports only GSM_PCHAN_OSMO_DYN.
      * So we need to deact only on this pchan kind. */

  • Patch Set #9, Line 974: See comment above)

    it is not clear what this refers to ... just drop this? or explicitly say what you mean to say.

    I think you mean

     /* BSC co-located PCU applies only to Ericsson RBS, which supports only GSM_PCHAN_OSMO_DYN.
      * So we need to act only on this pchan kind. */

• File src/osmo-bsc/timeslot_fsm.c:

  • Patch Set #9, Line 347: }

    it is very easy and quick to put the code move in a separate patch preceding this patch, so that the addition of 'is_ericsson_bts()' is not "hidden" in the moved code block. i would appreciate that, also for future readers of the git log. You can even +2 that patch yourself saying "trivial" -- because it would be only a cosmetic move of unchanged code. I could also do that for you if it helps...

  • Patch Set #9, Line 1128: NOTE

    i'm really puzzled why you seem to start every comment everywhere with "NOTE:" -- it is completely redundant and has no meaning at all

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