Attention is currently required from: falconia, manawyrm, pespin, roox.

View Change

4 comments:

• File include/osmocom/isdn/v110_ta.h:

  • Patch Set #13, Line 22: struct osmo_v110_ta_cfg {

    imho each of these should be set through separate APIs.

    If needed, we can always add API to [re]configure various parameters separately.
    For now I think it's acceptable to configure everything at once.

• File src/isdn/v110_ta.c:

  • Patch Set #13, Line 152: static inline bool v110_df_x_bits_are(const struct osmo_v110_decoded_frame *df, ubit_t cmp)

    are what? […]

    `what` is what you pass as `cmp` argument.

    See invocations of this function:

    ```
    $ git grep -n "v110_df_[sx]_bits_are"
    src/isdn/v110_ta.c:359: if (v110_df_s_bits_are(df, V110_SX_BIT_ON) &&
    src/isdn/v110_ta.c:360: v110_df_x_bits_are(df, V110_SX_BIT_ON)) {
    src/isdn/v110_ta.c:459: if (v110_df_s_bits_are(df, V110_SX_BIT_OFF) && v110_df_d_bits_are(df, 0)) {
    src/isdn/v110_ta.c:517: if (v110_df_s_bits_are(df, V110_SX_BIT_OFF)) {
    ```

    so it's similar to `lchan_state_is()` in osmo-bsc.git.

    • Decoded Frame S-bits are `V110_SX_BIT_{ON,OFF}`.
    • Decoded Frame X-bits are `V110_SX_BIT_{ON,OFF}`.

    Does that make sense to you?

  • Patch Set #13, Line 402: /* XXX: OSMO_ASSERT(V24_FLAGMASK_IS_ON(ts->v24_flags, OSMO_V110_TA_C_105)); */

    why are these commented out?

    Because I am not sure if it's correct to `assert()` here.
    It's likely fine to assert line 108/DTR, but I don't know about line 105/RTS.

  • Patch Set #13, Line 437: /* TODO: if (ta->cfg->flow_ctrl.end_to_end) { ... } */

    this is left for later? not important?

    Yes, this is left for later and thus TODO.

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