- gerrit-log - lists.osmocom.org

[M] Change in osmo-trx[master]: transceiver: add optional Laurent burst LUT modulation (int16)
by laforge 11 Mar '26

11 Mar '26

Attention is currently required from: Hoernchen, Timur Davydov, tnt. laforge has posted comments on this change by Timur Davydov. ( https://gerrit.osmocom.org/c/osmo-trx/+/42248?usp=email ) Change subject: transceiver: add optional Laurent burst LUT modulation (int16) ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: I can't really say muhc about this SDR topic, I'd appreciat the input of @tnt@246tNt.com and/or @ewild@sysmocom.de -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/42248?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: I1715c2d33dc55fe1c7be5c6e7259d378ea5e80b2 Gerrit-Change-Number: 42248 Gerrit-PatchSet: 5 Gerrit-Owner: Timur Davydov <dtv.comp(a)gmail.com> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: tnt <tnt(a)246tNt.com> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: tnt <tnt(a)246tNt.com> Gerrit-Attention: Timur Davydov <dtv.comp(a)gmail.com> Gerrit-Comment-Date: Wed, 11 Mar 2026 09:39:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in osmo-ccid-firmware[master]: ccid: Handle the TPDU-length = 4 case according to CCID spec
by laforge 11 Mar '26

11 Mar '26

laforge has posted comments on this change by laforge. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42317?usp=email ) Change subject: ccid: Handle the TPDU-length = 4 case according to CCID spec ...................................................................... Set Ready For Review -- To view, visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42317?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-ccid-firmware Gerrit-Branch: master Gerrit-Change-Id: Ia27962bda526af3373b36d637e2b548aa01a3346 Gerrit-Change-Number: 42317 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Wed, 11 Mar 2026 00:47:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No

1 0

[M] Change in osmo-trx[master]: build: add --disable-multi-arfcn configure option
by Timur Davydov 11 Mar '26

11 Mar '26

Attention is currently required from: Hoernchen, Timur Davydov, fixeria, laforge, osmith, pespin. Hello Hoernchen, Jenkins Builder, fixeria, laforge, osmith, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/42243?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: build: add --disable-multi-arfcn configure option ...................................................................... build: add --disable-multi-arfcn configure option - add configure flag to disable Multi ARFCN support - compile Channelizer, Synthesis, RadioInterfaceMulti and fft.c only when enabled - guard Multi ARFCN-specific code with ENABLE_MULTI_ARFCN - avoid requiring fftw3f when Multi ARFCN support is disabled Change-Id: I2495c5b5c79f2fe74bef83902058e1d2207c1f3c --- M Transceiver52M/Makefile.am M Transceiver52M/arch/common/Makefile.am M Transceiver52M/osmo-trx.cpp M Transceiver52M/radioInterface.h M configure.ac 5 files changed, 44 insertions(+), 11 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/43/42243/6 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/42243?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: I2495c5b5c79f2fe74bef83902058e1d2207c1f3c Gerrit-Change-Number: 42243 Gerrit-PatchSet: 6 Gerrit-Owner: Timur Davydov <dtv.comp(a)gmail.com> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: Timur Davydov <dtv.comp(a)gmail.com>

1 0

[M] Change in osmo-trx[master]: build: add --disable-multi-arfcn configure option
by Jenkins Builder 11 Mar '26

11 Mar '26

Attention is currently required from: Hoernchen, fixeria, laforge, osmith, pespin. Jenkins Builder has posted comments on this change by Timur Davydov. ( https://gerrit.osmocom.org/c/osmo-trx/+/42243?usp=email ) Change subject: build: add --disable-multi-arfcn configure option ...................................................................... Patch Set 5: (1 comment) File Transceiver52M/radioInterface.h: Robot Comment from checkpatch (run ID ): https://gerrit.osmocom.org/c/osmo-trx/+/42243/comment/379e71ef_934bb61f?usp… : PS5, Line 198: #endif /* ENABLE_MULTI_ARFCN */ adding a line without newline at end of file -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/42243?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: I2495c5b5c79f2fe74bef83902058e1d2207c1f3c Gerrit-Change-Number: 42243 Gerrit-PatchSet: 5 Gerrit-Owner: Timur Davydov <dtv.comp(a)gmail.com> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 11 Mar 2026 00:22:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in osmo-trx[master]: build: add --disable-multi-arfcn configure option
by Timur Davydov 11 Mar '26

11 Mar '26

Attention is currently required from: Hoernchen, fixeria, laforge, osmith, pespin. Hello Hoernchen, Jenkins Builder, fixeria, laforge, osmith, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-trx/+/42243?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: build: add --disable-multi-arfcn configure option ...................................................................... build: add --disable-multi-arfcn configure option - add configure flag to disable Multi ARFCN support - compile Channelizer, Synthesis, RadioInterfaceMulti and fft.c only when enabled - guard Multi ARFCN-specific code with ENABLE_MULTI_ARFCN - avoid requiring fftw3f when Multi ARFCN support is disabled Change-Id: I2495c5b5c79f2fe74bef83902058e1d2207c1f3c --- M Transceiver52M/Makefile.am M Transceiver52M/arch/common/Makefile.am M Transceiver52M/osmo-trx.cpp M Transceiver52M/radioInterface.h M configure.ac 5 files changed, 44 insertions(+), 11 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-trx refs/changes/43/42243/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/42243?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: I2495c5b5c79f2fe74bef83902058e1d2207c1f3c Gerrit-Change-Number: 42243 Gerrit-PatchSet: 5 Gerrit-Owner: Timur Davydov <dtv.comp(a)gmail.com> Gerrit-Reviewer: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[M] Change in osmo-ccid-firmware[master]: ccid: properly emit wait time ext messages
by Hoernchen 11 Mar '26

11 Mar '26

Attention is currently required from: laforge. Hoernchen has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42306?usp=email ) Change subject: ccid: properly emit wait time ext messages ...................................................................... Patch Set 2: (4 comments) File ccid_common/ccid_slot_fsm.c: https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42306/comment/b3dd17d6_9b… : PS2, Line 229: resp = ccid_gen_data_block(cs, ss->seq, CCID_CMD_STATUS_TIME_EXT, 1, NULL, 0); > can we be sure that we can allocate sufficient waitign time extension messages? what a about a car […] Then the firmware will crash and burn either way because we can't handle that slow timing and other slots won't have buffers either, but it's now a flag instead of a counter, processed once per loop, and the total amount is clamped until the card is considered mute. File ccid_common/iso7816_fsm.h: https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42306/comment/42e6f0f8_18… : PS2, Line 25: ISO7816_E_RX_SINGLE, /*!< single-byte data received on UART */ > this cosmetic re-formatting or code does not belong to the patch. […] Just unhide the build failure here and you can see that the linter prevents building this without reformatting according to the linter rules we have. File ccid_common/iso7816_fsm.c: https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42306/comment/e52236f9_1d… : PS2, Line 227: { ISO7816_E_TX_ERR_IND, "TX_ERR_IND" }, > same as above Done https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42306/comment/7871849d_7a… : PS2, Line 555: S(ISO7816_E_POWER_UP_IND), > and more cosmetic changes Done -- To view, visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42306?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-ccid-firmware Gerrit-Branch: master Gerrit-Change-Id: Ib69483d453a0e5ebb1bc1885a8f78790a0f10d70 Gerrit-Change-Number: 42306 Gerrit-PatchSet: 2 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Wed, 11 Mar 2026 00:19:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

1 0

[M] Change in osmo-ccid-firmware[master]: ccid: properly emit wait time ext messages
by Hoernchen 11 Mar '26

11 Mar '26

Attention is currently required from: Hoernchen, laforge. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42306?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review-1 by laforge, Verified+1 by Jenkins Builder Change subject: ccid: properly emit wait time ext messages ...................................................................... ccid: properly emit wait time ext messages The CCID message that tells the host that the card asked for a wait time extension was missing. This is a bit weird, because it breaks the 1:1 relationship between messages and responses that usually exists, nor does it consume a bSeq, so we just emit WT msgs as appropriate. It is bounded to prevent msgb exhaustion issues. Closes: OS#6458 Change-Id: Ib69483d453a0e5ebb1bc1885a8f78790a0f10d70 --- M ccid_common/ccid_device.h M ccid_common/ccid_slot_fsm.c M ccid_common/iso7816_fsm.c M ccid_common/iso7816_fsm.h 4 files changed, 119 insertions(+), 82 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ccid-firmware refs/changes/06/42306/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42306?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-ccid-firmware Gerrit-Branch: master Gerrit-Change-Id: Ib69483d453a0e5ebb1bc1885a8f78790a0f10d70 Gerrit-Change-Number: 42306 Gerrit-PatchSet: 3 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: Hoernchen <ewild(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org>

1 0

[XS] Change in osmo-ccid-firmware[master]: ccid: generate proper error for short messages
by Hoernchen 11 Mar '26

11 Mar '26

Hoernchen has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42318?usp=email ) Change subject: ccid: generate proper error for short messages ...................................................................... ccid: generate proper error for short messages Change-Id: I3bf0bebd361666a2f3d0341444415a6f25d2b4bf --- M ccid_common/ccid_device.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ccid-firmware refs/changes/18/42318/1 diff --git a/ccid_common/ccid_device.c b/ccid_common/ccid_device.c index 8a53860..7866d9f 100644 --- a/ccid_common/ccid_device.c +++ b/ccid_common/ccid_device.c @@ -836,8 +836,10 @@ short_msg: LOGP(DCCID, LOGL_ERROR, "Short CCID message received: %s; ignoring\n", msgb_hexdump(msg)); + resp = gen_err_resp(ch->bMessageType, ch->bSlot, get_icc_status(cs), ch->bSeq, + CCID_ERR_CMD_NOT_SUPPORTED); msgb_free(msg); - return -1; + return ccid_slot_send_unbusy(cs, resp); } /* Section 5.3.1 ABORT */ -- To view, visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42318?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-ccid-firmware Gerrit-Branch: master Gerrit-Change-Id: I3bf0bebd361666a2f3d0341444415a6f25d2b4bf Gerrit-Change-Number: 42318 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>

1 0

[M] Change in osmo-ccid-firmware[master]: firmware: fix msgb/list mixed irq access handling
by Hoernchen 11 Mar '26

11 Mar '26

Hoernchen has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42319?usp=email ) Change subject: firmware: fix msgb/list mixed irq access handling ...................................................................... firmware: fix msgb/list mixed irq access handling This fixes the mishandling of in_progress and the queues in the submit functions that are in part irq driven, too, and ensures nothing breaks by preventing concurrent access from irqs to the queues. Applying the same pattern to all functions/cases is fine either way. On top of all of tha the out ep was irq driven, so if submit_next_out fails it was stuck, now fixed by attempted submit from main loop, and of course the queue manipulation should be irq safe here as well. Change-Id: I9212bfa7688cb4d3161ba963b854225744128632 --- M sysmoOCTSIM/main.c 1 file changed, 43 insertions(+), 30 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ccid-firmware refs/changes/19/42319/1 diff --git a/sysmoOCTSIM/main.c b/sysmoOCTSIM/main.c index 92c5bbd..e1e83c7 100644 --- a/sysmoOCTSIM/main.c +++ b/sysmoOCTSIM/main.c @@ -159,19 +159,26 @@ static int submit_next_in(void) { struct usb_ep_q *ep_q = &g_ccid_s.in_ep; - struct msgb *msg; + struct msgb *msg = NULL; int rc; - if (ep_q->in_progress) - return 0; + CRITICAL_SECTION_ENTER() + if (!ep_q->in_progress) { + msg = msgb_dequeue(&ep_q->list); + if (msg) + ep_q->in_progress = msg; + } + CRITICAL_SECTION_LEAVE() - msg = msgb_dequeue_irqsafe(&ep_q->list); if (!msg) return 0; - ep_q->in_progress = msg; rc = ccid_df_write_in(msgb_data(msg), msgb_length(msg)); if (rc != ERR_NONE) { + CRITICAL_SECTION_ENTER() + ep_q->in_progress = NULL; + msgb_enqueue(&g_ccid_s.free_q, msg); + CRITICAL_SECTION_LEAVE() printf("EP %s failed: %d\r\n", ep_q->name, rc); return -1; } @@ -183,22 +190,27 @@ static int submit_next_irq(void) { struct usb_ep_q *ep_q = &g_ccid_s.irq_ep; - struct msgb *msg; + struct msgb *msg = NULL; int rc; - if (ep_q->in_progress) - return 0; + CRITICAL_SECTION_ENTER() + if (!ep_q->in_progress) { + msg = msgb_dequeue(&ep_q->list); + if (msg) + ep_q->in_progress = msg; + } + CRITICAL_SECTION_LEAVE() - msg = msgb_dequeue_irqsafe(&ep_q->list); if (!msg) return 0; - ep_q->in_progress = msg; rc = ccid_df_write_irq(msgb_data(msg), msgb_length(msg)); /* may return HALTED/ERROR/DISABLED/BUSY/ERR_PARAM/ERR_FUNC/ERR_DENIED */ if (rc != ERR_NONE) { - // ep_q->in_progress = msg; - // msgb_enqueue_irqsafe(&ep_q->list, msg); + CRITICAL_SECTION_ENTER() + ep_q->in_progress = NULL; + msgb_enqueue(&g_ccid_s.free_q, msg); + CRITICAL_SECTION_LEAVE() printf("EP %s failed: %d\r\n", ep_q->name, rc); return -1; } @@ -208,22 +220,29 @@ static int submit_next_out(void) { struct usb_ep_q *ep_q = &g_ccid_s.out_ep; - struct msgb *msg; + struct msgb *msg = NULL; int rc; - if (ep_q->in_progress) - return 0; + CRITICAL_SECTION_ENTER() + if (!ep_q->in_progress) { + msg = msgb_dequeue(&g_ccid_s.free_q); + if (msg) { + msgb_reset(msg); + ep_q->in_progress = msg; + } + } + CRITICAL_SECTION_LEAVE() - msg = msgb_dequeue_irqsafe(&g_ccid_s.free_q); if (!msg) - return -1; - msgb_reset(msg); - ep_q->in_progress = msg; + return 0; rc = ccid_df_read_out(msgb_data(msg), msgb_tailroom(msg)); if (rc != ERR_NONE) { + CRITICAL_SECTION_ENTER() + ep_q->in_progress = NULL; /* re-add to the list of free msgb's */ - llist_add_tail_at(&msg->list, &g_ccid_s.free_q); + msgb_enqueue(&g_ccid_s.free_q, msg); + CRITICAL_SECTION_LEAVE() return 0; } return 1; @@ -494,7 +513,7 @@ OSMO_ASSERT(msg); /* append to list of pending-to-be-handed messages */ - llist_add_tail_at(&msg->list, &g_ccid_s.in_ep.list); + msgb_enqueue_irqsafe(&g_ccid_s.in_ep.list, msg); submit_next_in(); return 0; } @@ -557,19 +576,12 @@ msgb_reset(msg); llist_add_tail_at(&msg->list, &g_ccid_s.free_q); } -#if 0 struct msgb *cur_msg = cur_epq->in_progress; if (cur_msg) { - /* - if (i == 2 && usb_d_ep_get_status(/*_ccid_df_funcd.func_ep_out*/ 2, 0) == USB_BUSY) - continue; - */ - msgb_reset(cur_msg); - llist_add_tail_at(&cur_msg->list, &g_ccid_s.free_q); + msgb_enqueue(&g_ccid_s.free_q, cur_msg); cur_epq->in_progress = NULL; } -#endif } SysTick->CTRL |= SysTick_CTRL_ENABLE_Msk; @@ -715,7 +727,8 @@ struct msgb *msg = msgb_alloc(300, "ccid"); OSMO_ASSERT(msg); /* return the message back to the queue of free message buffers */ - llist_add_tail_at(&msg->list, &g_ccid_s.free_q); + msgb_enqueue_irqsafe(&g_ccid_s.free_q, msg); } + submit_next_out(); } } -- To view, visit https://gerrit.osmocom.org/c/osmo-ccid-firmware/+/42319?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-ccid-firmware Gerrit-Branch: master Gerrit-Change-Id: I9212bfa7688cb4d3161ba963b854225744128632 Gerrit-Change-Number: 42319 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <ewild(a)sysmocom.de>

1 0

[L] Change in pysim[master]: personalization: add param_source.py, add batch.py
by neels 11 Mar '26

11 Mar '26

Attention is currently required from: dexter. neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/41845?usp=email ) Change subject: personalization: add param_source.py, add batch.py ...................................................................... Patch Set 6: (11 comments) Patchset: PS6: there will be a new patch set soon, thanks for the feedback! File pySim/esim/saip/batch.py: https://gerrit.osmocom.org/c/pysim/+/41845/comment/f24a7bae_34d7aa7a?usp=em… : PS6, Line 65: params: list[ParamAndSrc]=None, > why not use [] as default? ... IIRC the linter warns against passing empty lists as default argument, because in python there can arise an evil twin situation that is hard to see: like some functions change the arg in-place or something, i don't remember in detail. https://gerrit.osmocom.org/c/pysim/+/41845/comment/bd98a8ed_3caf0795?usp=em… : PS6, Line 80: self.params = params or [] > ... then you could write self.params = params here. […] (marking done, bc discussion will be above) https://gerrit.osmocom.org/c/pysim/+/41845/comment/32c6d044_21645ff6?usp=em… : PS6, Line 92: csv_columns = next(self.csv_rows) > In the above comment you say that self.csv_rows can also be a list. […] hm that's right, it has to be an iter(list), thx https://gerrit.osmocom.org/c/pysim/+/41845/comment/a3e376b3_476ec8e8?usp=em… : PS6, Line 97: csv_row = None > As far as I understand self. […] it is optional, configured by the caller, whether a CSV file is present. If yes, it will pass nonempty csv_rows and use at least one CsvSource. So the csv_rows != None case definitely has to be supported. File pySim/esim/saip/param_source.py: https://gerrit.osmocom.org/c/pysim/+/41845/comment/df80bc66_187a2e6c?usp=em… : PS6, Line 31: class ParamSource: > As far as I understand this is an abstract class. Maybe declare it as […] That is right. The ABC discussion is a slightly dangerous one, because I am pretty firm in my opinion against using it, though not everyone seems to agree with it. It would be great if I am allowed to work without ABC, or otherwise let's schedule a call, or i will hold a talk about ABC -- either way let's not list points for and against it here but rather talk in person? https://gerrit.osmocom.org/c/pysim/+/41845/comment/b655dbde_252b85c9?usp=em… : PS6, Line 42: so the user can enter '0000' to get a four digit random number.""" > If this should work as the bare minimum, we should also have some kind of dummy constructor that can […] Hmm. it's even worse, most subclasses completely replace this. I will make a better plan, thanks. https://gerrit.osmocom.org/c/pysim/+/41845/comment/4cd391b0_8a0ba5dd?usp=em… : PS6, Line 51: > Excess linebreak? (the other classes are only separated with one) yea you're right, sometimes i do sometimes i don't, thx. i think it's because this separates the base definitions from the actual implementations, but not important https://gerrit.osmocom.org/c/pysim/+/41845/comment/944bfb95_3efd540d?usp=em… : PS6, Line 95: last_value = int(last_str) if last_str is not None else "9" * len(first_str) > Here last_value is either an int or it is a string with an integer number. […] heh funny code =) you are right https://gerrit.osmocom.org/c/pysim/+/41845/comment/c8a453fc_9b4a59b5?usp=em… : PS6, Line 121: val = random.randbytes(self.num_digits // 2) # TODO secure random source? > As far as I understand this should return a bytearray but what you actually want is a hexstring. […] This uses the fact that all BinaryParameter can take a bytes as input value. We could convert to hexstring and then the BinaryParameter can convert back to binary, but by passing just the bytes we skip the two conversions. A thought could be this: most places now use a string value to pass parameter settings around, so if we define that a string is the preferred way, then i would change this; but the idea in ConfigurableParameter that I took over from the code I got when I started is that a bytes and BytesIO and hexstring all work, so i would leave the patch unchanged.. agreed? https://gerrit.osmocom.org/c/pysim/+/41845/comment/3a1f2a8f_e471a54c?usp=em… : PS6, Line 174: return val > You could just do if csv_row: return csv_row.get(self.csv_column) else: raise ... […] it's more like if not csv_row or not csv_row.get(self.csv_column): raise... return csv_row.get(self.csv_column) so i'm pretty sure this patch as it is is the most minimalistic way of writing this =) -- To view, visit https://gerrit.osmocom.org/c/pysim/+/41845?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I01ae40a06605eb205bfb409189fcd2b3a128855a Gerrit-Change-Number: 41845 Gerrit-PatchSet: 6 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 10 Mar 2026 22:20:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log