Attention is currently required from: fixeria.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email )
Change subject: client: collapse codecs[] and ptmap[]; allow codec variants
......................................................................
Patch Set 5:
(1 comment)
File src/libosmo-mgcp-client/mgcp_client.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/34899/comment/0ae25fd3_33ed68ea
PS5, Line 421: <=
> oh my goodness, thx. […]
Ah of course, it is the mgcp *client* library, which we use in osmo-bsc, -msg, -hnbgw, so I need to run *their* test suites to check this code.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed
Gerrit-Change-Number: 34899
Gerrit-PatchSet: 5
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 31 Oct 2023 23:52:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email )
Change subject: client: collapse codecs[] and ptmap[]; allow codec variants
......................................................................
Patch Set 5:
(1 comment)
File src/libosmo-mgcp-client/mgcp_client.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/34899/comment/b88ccba1_6fa16c1b
PS5, Line 421: <=
> shouldn't this be `>=`?
oh my goodness, thx.
wait a minute, what does that say about our test suite.
I did run these patches by ttcn3-hnbgw-tests!
This code path should *always* fail, so how did I not catch it.
Is this dead code? Do we not have a=rtpmap in our tests? i don't think that can be true.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed
Gerrit-Change-Number: 34899
Gerrit-PatchSet: 5
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 31 Oct 2023 23:51:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email )
Change subject: client: collapse codecs[] and ptmap[]; allow codec variants
......................................................................
Patch Set 5: Code-Review-1
(1 comment)
Patchset:
PS5:
I just realized an API compat problem, for example osmo-hnbgw uses the codecs[] array and fails to build with this patch.
Any ideas?
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34899?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I798e02c6663376d3d52f4a74fc4b32411ce95bed
Gerrit-Change-Number: 34899
Gerrit-PatchSet: 5
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 31 Oct 2023 23:40:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: daniel, fixeria, osmith.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34923?usp=email )
Change subject: sccp: Introduce test TC_cr_timeout_cc_too_late
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
For some reason this builds locally fine, but fails here or in docker:
SCCP_Templates.ttcn:184.10-80: error: There is no local or imported definition with name `ConvertASPAddressToEncodedAddress_opt_itu'
I'm doing the same as existing ConvertASPAddressToEncodedAddress_itu, so not sure what's going on :/
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34923?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: If4b53565f1fa19894ca24fa71e02ae7b1941411e
Gerrit-Change-Number: 34923
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 31 Oct 2023 17:17:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: daniel, osmith, pespin.
Hello Jenkins Builder, daniel, osmith,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34923?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by osmith, Verified+1 by Jenkins Builder
Change subject: sccp: Introduce test TC_cr_timeout_cc_too_late
......................................................................
sccp: Introduce test TC_cr_timeout_cc_too_late
Related: SYS#6602
Change-Id: If4b53565f1fa19894ca24fa71e02ae7b1941411e
---
M library/SCCP_Templates.ttcn
M sccp/SCCP_Tests_RAW.ttcn
2 files changed, 104 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/23/34923/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34923?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: If4b53565f1fa19894ca24fa71e02ae7b1941411e
Gerrit-Change-Number: 34923
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge.
dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/34897?usp=email )
Change subject: runtime: fix tracking of selected_adf
......................................................................
Patch Set 3:
(2 comments)
Patchset:
PS3:
> this fix works. […]
I have had a closer look to select and select_file in runtime.py now. I really had problems to understand what really happens with select and select_file. I have cleaned up the code now: https://gerrit.osmocom.org/c/pysim/+/34932
PS3:
> I now see that self.selected_adf refers to RuntimeState. […]
The change I did in RuntimeState is wrong I think. What we do there is probing for the applications on a low level. This is in the startup phase where the file system is created. We do not have to maintain the lchan state in this phase. We also can not use select or select_file either. I think it is ok to access the card from here directly. Also accessing self.selected_adf should not work. It should be self.lchan[0].selected_adf.
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34897?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: I4cc0c58ff887422b4f3954d35c8380ddc00baa1d
Gerrit-Change-Number: 34897
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 31 Oct 2023 16:36:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/pysim/+/34931?usp=email )
Change subject: filesystem: fix method build_select_path_to
......................................................................
filesystem: fix method build_select_path_to
The method build_select_path_to chops off the first element of the
current path. This is done to prevent re-selection of the first file in
the current path.
Unfortunately chopping off the first element in the current path does
not work properly in a situation when the current path points to the MF.
This would chop off the first and last element in the list and the for
loop below would run 0 times.
To fix this, let's keep the first element and chop it off from the
resulting path.
Related: OS#5418
Change-Id: Ia521a7ac4c25fd3a2bc8edffdc45ec89ba4b16eb
---
M pySim/filesystem.py
1 file changed, 23 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/31/34931/1
diff --git a/pySim/filesystem.py b/pySim/filesystem.py
index 5950ad1..800f2cc 100644
--- a/pySim/filesystem.py
+++ b/pySim/filesystem.py
@@ -143,7 +143,6 @@
cur_fqpath = self.fully_qualified_path_fobj()
target_fqpath = target.fully_qualified_path_fobj()
inter_path = []
- cur_fqpath.pop() # drop last element (currently selected file, doesn't need re-selection
cur_fqpath.reverse()
for ce in cur_fqpath:
inter_path.append(ce)
@@ -153,7 +152,7 @@
for te2 in target_fqpath[i+1:]:
inter_path.append(te2)
# we found our common ancestor
- return inter_path
+ return inter_path[1:]
return None
def get_mf(self) -> Optional['CardMF']:
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34931?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: Ia521a7ac4c25fd3a2bc8edffdc45ec89ba4b16eb
Gerrit-Change-Number: 34931
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newchange
dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/pysim/+/34932?usp=email )
Change subject: runtime: refactor file selection methods select and select_file
......................................................................
runtime: refactor file selection methods select and select_file
The implementation of the methods select and select_file of class
RuntimeLchan is a bit complex. We access the card directly in several
places which makes it difficult to track the state changes. We should
clean this up so that we call self.rs.card.select_adf_by_aid/
self.scc.select_file from a single place only.
This means that the method select uses the method select_file. This
results in a much cleaner implementation. We also should take care
that the important states that we track (selected_file, selected_adf,
etc.) are updated by a single private method. Since the update always
must happen after a select _select_post is a good place to do this.
Related: OS#6210
Change-Id: I9ae213f3b078983f3e6d4c11db38fdbe504c84f2
---
M pySim/runtime.py
1 file changed, 89 insertions(+), 41 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/32/34932/1
diff --git a/pySim/runtime.py b/pySim/runtime.py
index 8660724..feaa355 100644
--- a/pySim/runtime.py
+++ b/pySim/runtime.py
@@ -64,7 +64,7 @@
self.mf.add_file(f)
# go back to MF before the next steps (addon probing might have changed DF)
- self.card._scc.select_file('3F00')
+ self.lchan[0].select('3F00')
# add application ADFs + MF-files from profile
apps = self._match_applications()
@@ -110,6 +110,12 @@
# probe for those applications
for f in sorted(set(apps_profile) - set(apps_taken), key=str):
try:
+ # we can not use the lchan provided methods select, or select_file
+ # since those method work on an already finished file model. At
+ # this point we are still in the initialization process, so it is
+ # no problem when we access the card directly without caring about
+ # updating other states. For normal selects at runtime, the caller
+ # must use the lchan provided methods select or select_file!
data, sw = self.card.select_adf_by_aid(f.aid)
if sw == "9000":
print(" %s: %s" % (f.name, f.aid))
@@ -163,11 +169,14 @@
def __init__(self, lchan_nr: int, rs: RuntimeState):
self.lchan_nr = lchan_nr
self.rs = rs
+ self.scc = self.rs.card._scc.fork_lchan(lchan_nr)
+
+ # File reference data
self.selected_file = self.rs.mf
self.selected_adf = None
self.selected_file_fcp = None
self.selected_file_fcp_hex = None
- self.scc = self.rs.card._scc.fork_lchan(lchan_nr)
+
def add_lchan(self, lchan_nr: int) -> 'RuntimeLchan':
"""Add a new logical channel from the current logical channel. Just affects
@@ -246,9 +255,17 @@
raise ValueError(
"Cannot select unknown file by name %s, only hexadecimal 4 digit FID is allowed" % fid)
+ self._select_pre(cmd_app)
+
try:
+ # We access the card through the select_file method of the scc object.
+ # If we succeed, we know that the file exists and we may proceed with
+ # creating a new file at run time. In case the file does not exist, we
+ # just abort. The state on the card (selected file/application) wont't
+ # be changed, so we do not have to update any state in that case.
(data, sw) = self.scc.select_file(fid)
except SwMatchError as swm:
+ self._select_post(cmd_app)
k = self.interpret_sw(swm.sw_actual)
if not k:
raise(swm)
@@ -267,8 +284,8 @@
desc="elementary file, manually added at runtime")
self.selected_file.add_files([f])
- self.selected_file = f
- return select_resp, data
+
+ self._select_post(cmd_app, f, data)
def _select_pre(self, cmd_app):
# unregister commands of old file
@@ -276,7 +293,17 @@
for c in self.selected_file.shell_commands:
cmd_app.unregister_command_set(c)
- def _select_post(self, cmd_app):
+ def _select_post(self, cmd_app, file:CardFile = None, select_resp_data = None):
+ # we store some reference data (see above) about the currently selected file.
+ # This data must be updated after every select.
+ if file:
+ self.selected_file = file
+ if isinstance(file, CardADF):
+ self.selected_adf = file
+ if select_resp_data:
+ self.selected_file_fcp_hex = select_resp_data
+ self.selected_file_fcp = self.selected_file.decode_select_response(select_resp_data)
+
# register commands of new file
if cmd_app and self.selected_file.shell_commands:
for c in self.selected_file.shell_commands:
@@ -293,22 +320,31 @@
inter_path = self.selected_file.build_select_path_to(file)
if not inter_path:
raise RuntimeError('Cannot determine path from %s to %s' % (self.selected_file, file))
-
self._select_pre(cmd_app)
- for p in inter_path:
+ # be sure the variables that we pass to _select_post contain valid values.
+ selected_file = self.selected_file
+ data = self.selected_file_fcp_hex
+
+ for f in inter_path:
try:
- if isinstance(p, CardADF):
- (data, sw) = self.rs.card.select_adf_by_aid(p.aid, scc=self.scc)
- self.selected_adf = p
+ # We now directly accessing the card to perform the selection. This
+ # will change the state of the card, so we must take care to update
+ # the local state (lchan) as well. This is done in the method
+ # _select_port. It should be noted that the caller must always use
+ # the methods select_file or select. The caller must not access the
+ # card directly since this would lead into an incoherence of the
+ # card state and the state of the lchan.
+ if isinstance(f, CardADF):
+ (data, sw) = self.rs.card.select_adf_by_aid(f.aid, scc=self.scc)
else:
- (data, sw) = self.scc.select_file(p.fid)
- self.selected_file = p
+ (data, sw) = self.scc.select_file(f.fid)
+ selected_file = f
except SwMatchError as swm:
- self._select_post(cmd_app)
+ self._select_post(cmd_app, selected_file, data)
raise(swm)
- self._select_post(cmd_app)
+ self._select_post(cmd_app, f, data)
def select(self, name: str, cmd_app=None):
"""Select a file (EF, DF, ADF, MF, ...).
@@ -317,9 +353,11 @@
name : Name of file to select
cmd_app : Command Application State (for unregistering old file commands)
"""
+ # if any intermediate step failes, we must be able to go back where we were
+ prev_sel_file = self.selected_file
+
# handling of entire paths with multiple directories/elements
if '/' in name:
- prev_sel_file = self.selected_file
pathlist = name.split('/')
# treat /DF.GSM/foo like MF/DF.GSM/foo
if pathlist[0] == '':
@@ -327,41 +365,29 @@
try:
for p in pathlist:
self.select(p, cmd_app)
- return
+ return self.selected_file_fcp
except Exception as e:
- # if any intermediate step fails, go back to where we were
self.select_file(prev_sel_file, cmd_app)
raise e
+ # we are now in the directory where the target file is located
+ # so we can now refer to the get_selectables() method to get the
+ # file object and select it using select_file()
sels = self.selected_file.get_selectables()
if is_hex(name):
name = name.lower()
- self._select_pre(cmd_app)
+ try:
+ if name in sels:
+ f = sels[name]
+ self.select_file(sels[name], cmd_app)
+ else:
+ self.probe_file(name, cmd_app)
+ except Exception as e:
+ self.select_file(prev_sel_file, cmd_app)
+ raise e
- if name in sels:
- f = sels[name]
- try:
- if isinstance(f, CardADF):
- (data, sw) = self.rs.card.select_adf_by_aid(f.aid, scc=self.scc)
- else:
- (data, sw) = self.scc.select_file(f.fid)
- self.selected_file = f
- except SwMatchError as swm:
- k = self.interpret_sw(swm.sw_actual)
- if not k:
- raise(swm)
- raise RuntimeError("%s: %s - %s" % (swm.sw_actual, k[0], k[1]))
- select_resp = f.decode_select_response(data)
- else:
- (select_resp, data) = self.probe_file(name, cmd_app)
-
- # store the raw + decoded FCP for later reference
- self.selected_file_fcp_hex = data
- self.selected_file_fcp = select_resp
-
- self._select_post(cmd_app)
- return select_resp
+ return self.selected_file_fcp
def status(self):
"""Request STATUS (current selected file FCP) from card."""
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/34932?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: I9ae213f3b078983f3e6d4c11db38fdbe504c84f2
Gerrit-Change-Number: 34932
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-MessageType: newchange