Attention is currently required from: laforge.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/35499?usp=email )
Change subject: pySim-shell: Update manual with examples for using with eUICC ISD-R
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/35499?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I4a0acdad5c7478ee76f92c7610c0e2a5331dea46
Gerrit-Change-Number: 35499
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 08 Jan 2024 10:38:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: falconia, manawyrm, pespin, roox.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35444?usp=email )
Change subject: isdn: initial implementation of the V.110 TA
......................................................................
Patch Set 13:
(4 comments)
File include/osmocom/isdn/v110_ta.h:
https://gerrit.osmocom.org/c/libosmocore/+/35444/comment/4e9d3f88_b7042896
PS13, 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:
https://gerrit.osmocom.org/c/libosmocore/+/35444/comment/1828b293_cbab5410
PS13, 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?
https://gerrit.osmocom.org/c/libosmocore/+/35444/comment/3f6b0633_1387b40a
PS13, 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.
https://gerrit.osmocom.org/c/libosmocore/+/35444/comment/cef210c6_3c7368e3
PS13, 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 https://gerrit.osmocom.org/c/libosmocore/+/35444?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5716bd6fd0201ee7a7a29e72f775972cd374082f
Gerrit-Change-Number: 35444
Gerrit-PatchSet: 13
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: manawyrm <osmocom.account(a)tbspace.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: roox <mardnh(a)gmx.de>
Gerrit-Attention: manawyrm <osmocom.account(a)tbspace.de>
Gerrit-Attention: roox <mardnh(a)gmx.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 08 Jan 2024 10:29:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: falconia, fixeria, manawyrm, roox.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35444?usp=email )
Change subject: isdn: initial implementation of the V.110 TA
......................................................................
Patch Set 13: Code-Review+1
(4 comments)
File include/osmocom/isdn/v110_ta.h:
https://gerrit.osmocom.org/c/libosmocore/+/35444/comment/7933d841_8cf23139
PS13, Line 22: struct osmo_v110_ta_cfg {
imho each of these should be set through separate APIs.
File src/isdn/v110_ta.c:
https://gerrit.osmocom.org/c/libosmocore/+/35444/comment/bfbdcd28_c8eabbcc
PS13, Line 152: static inline bool v110_df_x_bits_are(const struct osmo_v110_decoded_frame *df, ubit_t cmp)
are what?
rename?
https://gerrit.osmocom.org/c/libosmocore/+/35444/comment/ce0ecfbe_01127ac7
PS13, Line 402: /* XXX: OSMO_ASSERT(V24_FLAGMASK_IS_ON(ts->v24_flags, OSMO_V110_TA_C_105)); */
why are these commented out?
https://gerrit.osmocom.org/c/libosmocore/+/35444/comment/73156dcd_ba31dbd2
PS13, Line 437: /* TODO: if (ta->cfg->flow_ctrl.end_to_end) { ... } */
this is left for later? not important?
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35444?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5716bd6fd0201ee7a7a29e72f775972cd374082f
Gerrit-Change-Number: 35444
Gerrit-PatchSet: 13
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: manawyrm <osmocom.account(a)tbspace.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: roox <mardnh(a)gmx.de>
Gerrit-Attention: manawyrm <osmocom.account(a)tbspace.de>
Gerrit-Attention: roox <mardnh(a)gmx.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 08 Jan 2024 09:50:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment