Attention is currently required from: dexter, fixeria, neels.
laforge has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39742?usp=email )
Change subject: personalization: refactor ConfigurableParameter, Iccid, Imsi
......................................................................
Patch Set 14: -Code-Review
(1 comment)
File pySim/esim/saip/personalization.py:
https://gerrit.osmocom.org/c/pysim/+/39742/comment/b7cf3efb_5b554be9?usp=em… :
PS13, Line 50: r"""Base class representing a part of the eSIM profile that is configurable during the
> abc has no purpose and it doesn't work.
My gist was that it doesn't solve exactly the problem you are trying to solve, but that doesn't mean it has no porpose and doesn't work.
> The first time this comment came up, I tried to use abc to determine which ConfigurableParameter classes are abstract. This did not work at all.
Sadly code review (particularly dragging on over the better part of a year so everyone forgets about what exactly was discussed before) is not a method of how you could demonstrate what exactly was not working in which way exactly. In such instances I would appreciate a "this is the branch containing what I tried, and it does not do XYZ while I'm trying to achieve ABC".
> Adding empty cruft to code is not what I do.
You sound like adding type annotations is also empty cruft?
If a class is not supposed to be used directly, but only inherited by other classes (one of which finally being none-abstract), it's an abstract base class. Adding the annotation by inheriting from abc.ABC shows that at the very least to whoever is reading the code, and ideally also allows the python runtime to raise meaningful exceptions if someone ever should try to instantiate the ABC directly. If you don't do that, people could just instantiate your class.
> the logical reason to change the patch is simply not present. I would love to accept your point, but explain to me why exactly you want to see abc in there.
The logical point is simply: there is a python standard library method of declaring "this class is abstract and shall not be instantiated directly". If your class falls into that category, it should be declared that way.
Furthermore, the code contains abc.abstractmethod decorator on a class that is not an ABC. According to the documentation this violates the abc module requirements:
```
A decorator indicating abstract methods. Using this decorator requires that the class’s metaclass is ABCMeta or is derived from it.
See https://docs.python.org/3/library/abc.html#abc.abstractmethod
```
> If the reason is cosmetic, then let me know.
> I will disagree and i will still have to use a simple boolean flag to make it work as it should,
You may need to do that, to solve your specific problem (different from what abc may solve). But to this point, after lots of code review I still don't fully understand the full picture, especially as I have no example/branch of "what doesn't work". I'm not trying to undestand everything in the review of this specific patch, but I'm merely saying
* if the class is abstract it should be labelled as such
* if you use abc.abstractmethod, the python standard library documentation says you must also mark the class as ABC
I'm not saying the above is solving all of your problems in the way you are trying to solve them.
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/39742?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: I6522be4c463e34897ca9bff2309b3706a88b3ce8
Gerrit-Change-Number: 39742
Gerrit-PatchSet: 14
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 08 Jan 2026 14:54:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Attention is currently required from: pespin.
laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41784?usp=email )
Change subject: bts: Validate no TIME.ind block gaps in TC_pcu_time_ind
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41784?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Ibce780009389b2cd06d1a6d79afa77d8e58187bc
Gerrit-Change-Number: 41784
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 08 Jan 2026 14:38:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: pespin.
laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41782?usp=email )
Change subject: bts: TC_pcu_{rts_req,time_ind}: process FNs in port queue after time out
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41782?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: Iacd4eafbdb42207465ac0c5e03492c460280ecf7
Gerrit-Change-Number: 41782
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 08 Jan 2026 14:38:28 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: pespin.
laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41781?usp=email )
Change subject: bts: Validate no PDTCH/PTCCH block gaps in TC_pcu_rts_req
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41781?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I0bf3540523c231ed7172cab720163816d5d81e26
Gerrit-Change-Number: 41781
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 08 Jan 2026 14:38:04 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: pespin.
laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41780?usp=email )
Change subject: Move function to calculate next PDTCH block FN to library
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41780?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I49289029c89367c8c6a5db7fafda099748ec15b6
Gerrit-Change-Number: 41780
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 08 Jan 2026 14:37:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41783?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by Jenkins Builder
Change subject: bts: TC_pcu_{rts_req,time_ind}: Relax expectancies on rx primitive count
......................................................................
bts: TC_pcu_{rts_req,time_ind}: Relax expectancies on rx primitive count
Even with latest changes I can still sometimes run into up to 6 FNs less
than the currently minimum expected. That's usually 1-2 primitives less,
which in the number of >100 we are receiving over 5 seconds, seems like
a plausible drift.
Change-Id: I887bd80a90e3ef8142cc29acde2ac9a3ea4869d6
---
M bts/BTS_Tests.ttcn
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/83/41783/3
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41783?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I887bd80a90e3ef8142cc29acde2ac9a3ea4869d6
Gerrit-Change-Number: 41783
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder