laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/24401 )
Change subject: RAW_NS/NS_Provider_IPL4: allow to use the new NSVC interface ...................................................................... Patch Set 2: (3 comments) to be honest, I'm overall not very convinced by this patch. It looks more like a kludge/hack than a proper solution. In general, we should try to keep code as generic as possible and not include too many special use cases. If there is a common way to interface the NS_Provider with its consumer, then we should go for that rather than supporting two different ways. Ideally we also wouldn't have a new un-conditional dependency to NS_RAW from the NS_Provider_IPL4. If it's absolutely neccessary, one c ould think of #ifdef'ing it out and using ttcnpp, like we e.g. do in IPA_Emulation to avoid extra dependencies that otherwise are not there. Without spending more time on thinking about this in detail, I don't immediately have a better solution, but I think there should be some way to have a general interface betwene the provider and the consumer, without special-casing between the two existing consumers. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/24401/2/library/NS_Provider_IPL4.ttcn File library/NS_Provider_IPL4.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/24401/2/library/NS_Provider_IPL4.ttcn@57 PS2, Line 57: integer vc_raw_idx as those two are mutually exclusive, one could introduce a union here to show those are two mutually excluded alternatives code can then use ischosen() to determine if either of the union alternatives has been selected or not (rather than checking for a component reference being null). https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/24401/2/library/NS_Provider_IPL4.ttcn@60 PS2, Line 60: sig AFAICT, adding the raw NSVC is really different from the "normal" one, so it might make sense to keep the old signature as-is and add a second NSPIP_add_nsvc_raw() https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/24401/2/library/NS_Provider_IPL4.ttcn@76 PS2, Line 76: connect(self:NSVC[i], nsvc.vc_raw:NSCP[nsvc.vc_raw_idx]); so the only difference between those two is that one has an array, and the other has not? this needs more thought. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/24401 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: Iafd9310e04066958914201da0cbdcd563bd5c976 Gerrit-Change-Number: 24401 Gerrit-PatchSet: 2 Gerrit-Owner: lynxis lazus <lyn...@fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillm...@sysmocom.de> Gerrit-CC: laforge <lafo...@osmocom.org> Gerrit-Comment-Date: Sun, 30 May 2021 09:31:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
