March 2025 - gerrit-log - lists.osmocom.org

[M] Change in pysim[master]: [2/6] personalization: refactor ConfigurableParameter, Iccid, Imsi

by neels

Attention is currently required from: dexter, fixeria. neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39742?usp=email ) Change subject: [2/6] personalization: refactor ConfigurableParameter, Iccid, Imsi ...................................................................... Patch Set 6: (1 comment) File pySim/esim/saip/personalization.py: https://gerrit.osmocom.org/c/pysim/+/39742/comment/510ae754_497088fe?usp=em… : PS6, Line 117: If self.value is None, first call self.validate() to generate a sanitized self.value from self.input_value. > This comment reads like if it were the API users task to call self.validate if self.value is none. […] I see how this creates a misunderstanding: I always write API docs in the "imperative mood", which is like instructing someone who is implementing the function. Like ~~~ Iterate all numbers from 0 to max and return a list of prime numbers in that range. ~~~ Because this form is concise, the shortest possible form, compare: ~~~ This function interates all numbers from 0 to max and then returns a list of prime numbes in that range. ~~~ But this may sound like the caller should do it: ~~~ first call self.validate() ~~~ The intention was the opposite. I guess this is a case where the imperative mood doesn't work so well... ~~~ Implicitly call self.validate() when self.value is None. ~~~ kind of works, but: ~~~ self.validate() is called implicitly when... ~~~ thanks! -- 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: 6 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-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 13 Mar 2025 22:08:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>

3 months, 3 weeks

1
0
0 0

[M] Change in pysim[master]: [1/6] personalization: refactor: drop ClassVarMeta use

by neels

Attention is currently required from: fixeria, laforge. neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39741?usp=email ) Change subject: [1/6] personalization: refactor: drop ClassVarMeta use ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > I'm not entirely sure why the new approach is considered better or an advantage over the old one. […] Name: Ah, I think this is an artifact from polishing and rebasing the commits on my branch. It seems this patch is removing the 'name', and then 'name' is added back in a commit still waiting on my branch. The intention was to have a slight tweak of 'name', all in one commit: to keep the autmatic name as it is now, but also have the option of using an explicit string name in a subclass, so we're not limited to py syntax. 'name' will definitely stay! About why the branch is even touching the name part: I was at first working with the class names themselves (the automatic 'name'), but figured that the typical format is, for example, "PIN1", not "Pin1". For the SdKey subclasses, we probably will want names that are more complex than python class name syntax permits. We might also want to rename class K to a more normal class syntax like AkaKey? Anyway IMHO it is better to keep the personalization API+UI namespace separate from python code internals. CSV matching: I do use the 'name' in our web platform code to match CSV columns to parameter names -- but only to aid the user with picking which column to feed to which parameter. The CSV column matching code so far is intended to not migrate to the pysim repos; pysim will have all the code for feeding explicitly selected CSV columns to explicitly selected parameters, only. But I guess we could also move that automatic column matching code here to pysim.git in some way or other. (For that matching, it does not matter if the name is "Pin1" or "PIN1" or even "~pin: 1~", it is comparing only lowercased and sanitized to [a-z0-9]) "Approach": you mean using ClassVarMeta is better than plain standard python? If yes then I disagree, otherwise I am not comprehending and please elaborate. My opinion against ClassVarMeta is that future patches add more members, and I do not think it is a good idea to add all those as kwargs fed to a messed-with `__new__()` builtin. Plain class members work just as well AFAICT, so i do want to stick with plain python and proper inheritance and normal access to class members where one is initialized from the value of the other like ~~~ class Pin(ConfigurableParameter): min_len = 4 default_value = '0' * min_len ~~~ -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39741?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: I60ea8fd11fb438ec90ddb08b17b658cbb789c051 Gerrit-Change-Number: 39741 Gerrit-PatchSet: 6 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 13 Mar 2025 21:52:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

3 months, 3 weeks

1
0
0 0

[M] Change in pysim[master]: [4/6] personalization: refactor Pin, Adm

by neels

Attention is currently required from: laforge. neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39744?usp=email ) Change subject: [4/6] personalization: refactor Pin, Adm ...................................................................... Patch Set 3: (2 comments) File pySim/esim/saip/personalization.py: https://gerrit.osmocom.org/c/pysim/+/39744/comment/3f2c4e54_a9a4b3ed?usp=em… : PS3, Line 408: return (pe for pe in l if pe.type == wanted_type) > ProfileElementSequence.pe_by_type, yes! and also ProfileElementSequence. […] No! First, apply_val() looks up ~~~ pes.pes_by_naa['mf'] ~~~ The result looks like this: ~~~ >>> mf = pes.pes_by_naa['mf'] >>> pprint.pprint(mf) [[<pySim.esim.saip.ProfileElementMF object at 0x7fcd2285e3c0>, <pySim.esim.saip.ProfileElementPuk object at 0x7fcd2285e900>, <pySim.esim.saip.ProfileElementPin object at 0x7fcd2285ea50>, <pySim.esim.saip.ProfileElementTelecom object at 0x7fcd2285eba0>, <pySim.esim.saip.ProfileElementGFM object at 0x7fcd2285ee40>, <pySim.esim.saip.ProfileElementPin object at 0x7fcd22856850>, <pySim.esim.saip.ProfileElementGFM object at 0x7fcd22856990>]] ~~~ It then picks the first element [0] which is now a ~~~ >>> type(mf[0]) <class 'list'> ~~~ So now we have just a py list of various ProfileElement subclass instances. Not a ProfileElementSequence. I can do ~~~ >>> all_pincodes = pes.pe_by_type['pinCodes'] >>> pprint.pprint(all_pincodes) [<pySim.esim.saip.ProfileElementPin object at 0x7fcd2285ea50>, <pySim.esim.saip.ProfileElementPin object at 0x7fcd22856850>, <pySim.esim.saip.ProfileElementPin object at 0x7fcd22856ad0>, <pySim.esim.saip.ProfileElementPin object at 0x7fcd22845cd0>, <pySim.esim.saip.ProfileElementPin object at 0x7fcd22845940>] ~~~ But the result is different from the code I found, which looks up only the 'mf' naa: ~~~ >>> mf_pincodes = list(pe for pe in mf[0] if pe.type == 'pinCodes') >>> pprint.pprint(mf_pincodes) [<pySim.esim.saip.ProfileElementPin object at 0x7fcd2285ea50>, <pySim.esim.saip.ProfileElementPin object at 0x7fcd22856850>] ~~~ There are two questions for me: 1. Is it correct to limit to 'mf'? Until now I didn't know there were other pinCodes in other naas too. (I did not create this code, just working with code I got) 2. the pySim API: it seems slightly quirky design to have a ProfileElementSequence class, that in turn contains plain lists. That way those lookup members of ProfileElementSequence are not available in inner list levels. Would it make sense to use ProfileElementSequence instances on all inner lists, too? I think not, because it falls out of the asn1 decoding like this, right? Does it maybe make sense to have *only* global lookup functions that work on both ProfileElementSequence and list(ProfileElement) to avoid code dup? that would then defy those lookup dicts. Then again, are those dicts important, like, do they dramatically decrease lookup time? If we answer 1. with yes, limit to 'mf', and 2 with no, it's fine just as it is, then this patch is also fine as it is -- besides maybe moving the new function to a more prominent place. What are your thoughts? Thanks! https://gerrit.osmocom.org/c/pysim/+/39744/comment/7d1ea426_9bf78ee5?usp=em… : PS3, Line 452: def _apply_pinvalue(pe: ProfileElement, keyReference, val_bytes): > so according to this type annotation, 'pe' is a single ProfileElement. […] it is indeed a List[ProfileElement] as i found out now .. related to my long comment. closing this speech bubble so we have one place to discuss. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39744?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: I54aef10b6d4309398d4b779a3740a7d706d68603 Gerrit-Change-Number: 39744 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 13 Mar 2025 21:15:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

3 months, 3 weeks

1
0
0 0

[M] Change in pysim[master]: [4/6] personalization: refactor Pin, Adm

by neels

Attention is currently required from: laforge. neels has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39744?usp=email ) Change subject: [4/6] personalization: refactor Pin, Adm ...................................................................... Patch Set 3: (1 comment) File pySim/esim/saip/personalization.py: https://gerrit.osmocom.org/c/pysim/+/39744/comment/ff328eb7_29d500b2?usp=em… : PS3, Line 408: return (pe for pe in l if pe.type == wanted_type) > this seems a bit like a reimplementation of the ProfileElementList. […] ProfileElementSequence.pe_by_type, yes! and also ProfileElementSequence.get_pes_for_type(), as i see now. Thanks, missed those for some reason back when i started. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39744?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: I54aef10b6d4309398d4b779a3740a7d706d68603 Gerrit-Change-Number: 39744 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 13 Mar 2025 20:45:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

3 months, 3 weeks

1
0
0 0

[M] Change in libosmo-abis[master]: trau: new function osmo_trau2rtp_ufe()

by falconia

falconia has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/39731?usp=email ) Change subject: trau: new function osmo_trau2rtp_ufe() ...................................................................... trau: new function osmo_trau2rtp_ufe() TRAU frame formats for newer codecs (HR, EFR, AMR) include a UFE (uplink frame error) indicator bit in the DL direction. TRAUs are expected to set this bit to active error state (0 for HR/EFR/AMR or 1 for D144 E-TRAU) if frame synchronization is lost in the UL direction, or if received TRAU-UL frames have bad CRC or invalid control bits - errors that indicate corruption on terrestrial circuits or faults in the CCU, rather than radio Rx errors. In Osmocom implementation, TRAU frame CRC and control bit patterns are checked during conversion to RTP, as part of the work done by osmo_trau2rtp() - but there is no separate return indication from that function that distinguishes these errors from ordinary BFIs due to radio errors or FACCH stealing etc. Solution: add a new API function that indicates UFE conditions separately. Related: OS#6614 Change-Id: I7a90eca296b17b1211e5c2125fb1496cd362e1c9 --- M include/osmocom/trau/trau_rtp.h M src/trau/trau_rtp_conv.c 2 files changed, 90 insertions(+), 33 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified fixeria: Looks good to me, approved laforge: Looks good to me, but someone else must approve diff --git a/include/osmocom/trau/trau_rtp.h b/include/osmocom/trau/trau_rtp.h index 3631b83..cb6d8e2 100644 --- a/include/osmocom/trau/trau_rtp.h +++ b/include/osmocom/trau/trau_rtp.h @@ -32,5 +32,8 @@ int osmo_trau2rtp(uint8_t *out, size_t out_len, const struct osmo_trau_frame *tf, struct osmo_trau2rtp_state *st); +int osmo_trau2rtp_ufe(uint8_t *out, size_t out_len, const struct osmo_trau_frame *tf, + struct osmo_trau2rtp_state *st, bool *ufe); + int osmo_rtp2trau(struct osmo_trau_frame *tf, const uint8_t *rtp, size_t rtp_len, struct osmo_trau2rtp_state *st); diff --git a/src/trau/trau_rtp_conv.c b/src/trau/trau_rtp_conv.c index 8307112..a36d317 100644 --- a/src/trau/trau_rtp_conv.c +++ b/src/trau/trau_rtp_conv.c @@ -134,6 +134,7 @@ * \param[out] out caller-provided output buffer * \param[in] out_len length of out buffer in bytes * \param[in] fr input TRAU frame in decoded form + * \param[in] emit_twts001 Generate RTP in TW-TS-001 format * \returns number of bytes generated in 'out'; negative on error. */ static int trau2rtp_fr(uint8_t *out, size_t out_len, const struct osmo_trau_frame *tf, bool emit_twts001) { @@ -245,8 +246,12 @@ * \param[out] out caller-provided output buffer * \param[in] out_len length of out buffer in bytes * \param[in] tf input TRAU frame in decoded form + * \param[in] emit_twts002 Generate RTP in TW-TS-002 format + * \param[out] ufe If not NULL: must be initialized to false by the user, + * set to true by the function if an Uplink Frame Error is detected * \returns number of bytes generated in 'out'; negative on error. */ -static int trau2rtp_hr16(uint8_t *out, size_t out_len, const struct osmo_trau_frame *tf, bool emit_twts002) +static int trau2rtp_hr16(uint8_t *out, size_t out_len, const struct osmo_trau_frame *tf, + bool emit_twts002, bool *ufe) { enum osmo_gsm631_sid_class sidc; int rc; @@ -274,7 +279,7 @@ * BFI or invalid SID packets. */ if (!emit_twts002 && (tf->c_bits[11] || sidc == OSMO_GSM631_SID_CLASS_INVALID)) - goto bad_frame; + return 0; /* BFI turns valid SID into invalid */ if (tf->c_bits[11] && sidc == OSMO_GSM631_SID_CLASS_VALID) @@ -306,6 +311,8 @@ return GSM_HR_BYTES_RTP_RFC5993; bad_frame: + if (ufe) + *ufe = true; if (emit_twts002) { out[0] = FT_NO_DATA << 4; twts002_hr16_set_extra_flags(out, tf); @@ -340,10 +347,13 @@ * \param[out] out caller-provided output buffer * \param[in] out_len length of out buffer in bytes * \param[in] tf input TRAU frame in decoded form - * \param[in] emit_twts002 self-explanatory + * \param[in] emit_twts002 Generate RTP in TW-TS-002 format + * \param[out] ufe If not NULL: must be initialized to false by the user, + * set to true by the function if an Uplink Frame Error is detected * \returns number of bytes generated in 'out'; negative on error. */ static int trau2rtp_hr8(uint8_t *out, size_t out_len, - const struct osmo_trau_frame *tf, bool emit_twts002) + const struct osmo_trau_frame *tf, bool emit_twts002, + bool *ufe) { unsigned xc1_4; @@ -409,6 +419,8 @@ default: bad_frame: /* received garbage, emit BFI with no data */ + if (ufe) + *ufe = true; if (emit_twts002) { out[0] = FT_NO_DATA << 4; twts002_hr8_set_extra_flags(out, tf); @@ -454,8 +466,12 @@ * \param[out] out caller-provided output buffer * \param[in] out_len length of out buffer in bytes * \param[in] fr input TRAU frame in decoded form + * \param[in] emit_twts001 Generate RTP in TW-TS-001 format + * \param[out] ufe If not NULL: must be initialized to false by the user, + * set to true by the function if an Uplink Frame Error is detected * \returns number of bytes generated in 'out'; negative on error. */ -static int trau2rtp_efr(uint8_t *out, size_t out_len, const struct osmo_trau_frame *tf, bool emit_twts001) +static int trau2rtp_efr(uint8_t *out, size_t out_len, const struct osmo_trau_frame *tf, + bool emit_twts001, bool *ufe) { size_t req_out_len = emit_twts001 ? GSM_EFR_BYTES+1 : GSM_EFR_BYTES; int i, j, rc; @@ -482,6 +498,33 @@ *twts001_hdr |= 0x01; } + /* verify CRC before proceeding with conversion to RTP */ + efr_parity_bits_1(check_bits, tf->d_bits); + rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 26, + tf->d_bits + 39); + if (rc) + goto bad_frame; + efr_parity_bits_2(check_bits, tf->d_bits); + rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 12, + tf->d_bits + 95); + if (rc) + goto bad_frame; + efr_parity_bits_3(check_bits, tf->d_bits); + rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 8, + tf->d_bits + 148); + if (rc) + goto bad_frame; + efr_parity_bits_4(check_bits, tf->d_bits); + rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 12, + tf->d_bits + 204); + if (rc) + goto bad_frame; + efr_parity_bits_5(check_bits, tf->d_bits); + rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 8, + tf->d_bits + 257); + if (rc) + goto bad_frame; + if (tf->c_bits[11] && !emit_twts001) /* BFI without TW-TS-001 */ return 0; @@ -490,39 +533,14 @@ /* reassemble d-bits */ for (i = 1, j = 4; i < 39; i++, j++) out[j/8] |= (tf->d_bits[i] << (7-(j%8))); - efr_parity_bits_1(check_bits, tf->d_bits); - rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 26, - tf->d_bits + 39); - if (rc) - goto bad_frame; for (i = 42, j = 42; i < 95; i++, j++) out[j/8] |= (tf->d_bits[i] << (7-(j%8))); - efr_parity_bits_2(check_bits, tf->d_bits); - rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 12, - tf->d_bits + 95); - if (rc) - goto bad_frame; for (i = 98, j = 95; i < 148; i++, j++) out[j/8] |= (tf->d_bits[i] << (7-(j%8))); - efr_parity_bits_3(check_bits, tf->d_bits); - rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 8, - tf->d_bits + 148); - if (rc) - goto bad_frame; for (i = 151, j = 145; i < 204; i++, j++) out[j/8] |= (tf->d_bits[i] << (7-(j%8))); - efr_parity_bits_4(check_bits, tf->d_bits); - rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 12, - tf->d_bits + 204); - if (rc) - goto bad_frame; for (i = 207, j = 198; i < 257; i++, j++) out[j/8] |= (tf->d_bits[i] << (7-(j%8))); - efr_parity_bits_5(check_bits, tf->d_bits); - rc = osmo_crc8gen_check_bits(&gsm0860_efr_crc3, check_bits, 8, - tf->d_bits + 257); - if (rc) - goto bad_frame; /* * Many BTS models will emit the previous content of their internal @@ -557,6 +575,8 @@ return req_out_len; bad_frame: + if (ufe) + *ufe = true; if (emit_twts001) { *twts001_hdr |= 0x06; /* BFI and No_Data flags */ return 1; @@ -1656,13 +1676,47 @@ int osmo_trau2rtp(uint8_t *out, size_t out_len, const struct osmo_trau_frame *tf, struct osmo_trau2rtp_state *st) { + return osmo_trau2rtp_ufe(out, out_len, tf, st, NULL); +} + +/*! convert received TRAU-UL frame to RTP payload, while also flagging + * Uplink Frame Errors. + * \param[out] out Buffer for the output RTP payload + * \param[in] out_len Size of buffer pointed to by \ref out + * \param[in] tf Osmo-decoded TRAU frame + * \param[in] st State/config structure + * \param[out] ufe If not NULL: must be initialized to false by the user, + * set to true by the function if an Uplink Frame Error is detected + * \returns length of converted RTP payload if successful; negative on error + * + * This function is just like plain osmo_trau2rtp(), but it also returns a + * secondary output: is the decoded TRAU-UL frame in error in terms of CRC + * or invalid control bits? This UFE flag is intended for use by software + * implementations of standard TRAU functionality, where a TRAU is expected + * to indicate UFE in its DL output if the UL input is corrupted in a way + * that is independent of radio Rx errors. + * + * Caveats: + * + * - *ufe is only set to true by this function, and is not written to at all + * if there is no error - thus the caller must initialize its bool variable + * to false before calling this API. + * + * - UFE checks exist only for HRv1 and EFR speech frames; for all other frame + * types, this function never writes to *ufe. (AMR is another frame type + * for which TRAU-UL decoding would include a UFE check, but we currently + * don't support AMR at all.) + */ +int osmo_trau2rtp_ufe(uint8_t *out, size_t out_len, const struct osmo_trau_frame *tf, + struct osmo_trau2rtp_state *st, bool *ufe) +{ switch (tf->type) { case OSMO_TRAU16_FT_FR: return trau2rtp_fr(out, out_len, tf, check_twts001(st)); case OSMO_TRAU16_FT_EFR: - return trau2rtp_efr(out, out_len, tf, check_twts001(st)); + return trau2rtp_efr(out, out_len, tf, check_twts001(st), ufe); case OSMO_TRAU16_FT_HR: - return trau2rtp_hr16(out, out_len, tf, check_twts002(st)); + return trau2rtp_hr16(out, out_len, tf, check_twts002(st), ufe); case OSMO_TRAU16_FT_DATA: return trau2rtp_data_fr(out, out_len, tf); case OSMO_TRAU16_FT_DATA_HR: @@ -1670,7 +1724,7 @@ case OSMO_TRAU16_FT_EDATA: return trau2rtp_edata(out, out_len, tf); case OSMO_TRAU8_SPEECH: - return trau2rtp_hr8(out, out_len, tf, check_twts002(st)); + return trau2rtp_hr8(out, out_len, tf, check_twts002(st), ufe); case OSMO_TRAU8_DATA: return trau2rtp_data_hr8(out, out_len, tf); default: -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/39731?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I7a90eca296b17b1211e5c2125fb1496cd362e1c9 Gerrit-Change-Number: 39731 Gerrit-PatchSet: 3 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

3 months, 3 weeks

1
0
0 0

[M] Change in libosmo-abis[master]: trau: new function osmo_trau2rtp_ufe()

by fixeria

Attention is currently required from: falconia, pespin. fixeria has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/libosmo-abis/+/39731?usp=email ) Change subject: trau: new function osmo_trau2rtp_ufe() ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/39731?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I7a90eca296b17b1211e5c2125fb1496cd362e1c9 Gerrit-Change-Number: 39731 Gerrit-PatchSet: 3 Gerrit-Owner: falconia <falcon(a)freecalypso.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: falconia <falcon(a)freecalypso.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 13 Mar 2025 20:02:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

3 months, 3 weeks

1
0
0 0

[S] Change in osmo-pcu[master]: csn1: Use enum to select enc/dec direction

by pespin

pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/osmo-pcu/+/39778?usp=email ) Change subject: csn1: Use enum to select enc/dec direction ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Same patch for libosmo-gprs here: https://gerrit.osmocom.org/c/libosmo-gprs/+/39783 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/39778?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I845bcab61e354436bff1c3a0f2b6f49de9705716 Gerrit-Change-Number: 39778 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 13 Mar 2025 19:02:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

3 months, 3 weeks

1
0
0 0

[S] Change in libosmo-gprs[master]: csn1: Use enum to select enc/dec direction

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/39783?usp=email ) Change subject: csn1: Use enum to select enc/dec direction ...................................................................... csn1: Use enum to select enc/dec direction Ported from osmo-pcu.git Change-Id I845bcab61e354436bff1c3a0f2b6f49de9705716 Change-Id: I216d4d94f585dd4dcf9947324fdbdc786ab2b5c5 --- M include/osmocom/csn1/csn1.h M src/csn1/csn1.c M src/csn1/csn1_dec.c M src/csn1/csn1_enc.c M src/rlcmac/csn1_ts_24_008.c M src/rlcmac/csn1_ts_44_060.c 6 files changed, 12 insertions(+), 9 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/83/39783/1 diff --git a/include/osmocom/csn1/csn1.h b/include/osmocom/csn1/csn1.h index a87f5a0..cb3ad13 100644 --- a/include/osmocom/csn1/csn1.h +++ b/include/osmocom/csn1/csn1.h @@ -52,7 +52,10 @@ #define CSNCBS_REVISION_LIMIT_STOP -20 /* Stop packing/unpacking - revision limit */ #define CSNCBS_NOT_SUPPORTED_IE -21 /* Handling of the unpacked IE is not supported by MS-software */ - +enum CSN_Direction { + CSN_DIRECTION_ENC = 0, + CSN_DIRECTION_DEC = 1, +}; #ifndef ElementsOf #define ElementsOf(array) (sizeof(array) / sizeof(array[0])) @@ -64,7 +67,7 @@ { gint remaining_bits_len; /* IN to an csn stream operation */ gint bit_offset; /* IN/OUT to an csn stream operation */ - gint direction; /* 0 - decode; 1 - encode */ + enum CSN_Direction direction; /* encode vs decode */ } csnStream_t; typedef gint16 (*StreamSerializeFcn_t)(csnStream_t* ar, struct bitvec *vector, unsigned *readIndex, void* data); diff --git a/src/csn1/csn1.c b/src/csn1/csn1.c index f07c414..5eb35ea 100644 --- a/src/csn1/csn1.c +++ b/src/csn1/csn1.c @@ -72,7 +72,7 @@ { ar->remaining_bits_len = remaining_bits_len; ar->bit_offset = bit_offset; - ar->direction = 0; + ar->direction = CSN_DIRECTION_ENC; } static const struct value_string csn1_error_names[] = { diff --git a/src/csn1/csn1_dec.c b/src/csn1/csn1_dec.c index 0fa09ad..c179f9b 100644 --- a/src/csn1/csn1_dec.c +++ b/src/csn1/csn1_dec.c @@ -483,7 +483,7 @@ remaining_bits_len -= length_len; osmo_csn1_stream_init(&arT, bit_offset, length > 0 ? length : remaining_bits_len); - arT.direction = 1; + arT.direction = CSN_DIRECTION_DEC; LOGPC(DLCSN1, LOGL_DEBUG, "offset = %u | ", pDescr->offset); Status = serialize(&arT, vector, readIndex, pvDATA(data, pDescr->offset)); diff --git a/src/csn1/csn1_enc.c b/src/csn1/csn1_enc.c index 6405525..a07caf0 100644 --- a/src/csn1/csn1_enc.c +++ b/src/csn1/csn1_enc.c @@ -433,7 +433,7 @@ *writeIndex += length_len; bit_offset += length_len; remaining_bits_len -= length_len; - arT.direction = 0; + arT.direction = CSN_DIRECTION_ENC; osmo_csn1_stream_init(&arT, bit_offset, remaining_bits_len); Status = serialize(&arT, vector, writeIndex, pvDATA(data, pDescr->offset)); diff --git a/src/rlcmac/csn1_ts_24_008.c b/src/rlcmac/csn1_ts_24_008.c index d38185d..79deff3 100644 --- a/src/rlcmac/csn1_ts_24_008.c +++ b/src/rlcmac/csn1_ts_24_008.c @@ -281,7 +281,7 @@ static gint16 Content_Dissector(csnStream_t* ar, struct bitvec *vector, unsigned *readIndex, void* data) { - if (ar->direction == 0) + if (ar->direction == CSN_DIRECTION_ENC) { return osmo_csn1_stream_encode(ar, CSNDESCR(Content_t), vector, readIndex, data); } @@ -305,7 +305,7 @@ static gint16 Additional_access_technologies_Dissector(csnStream_t* ar, struct bitvec *vector, unsigned *readIndex, void* data) { - if (ar->direction == 0) + if (ar->direction == CSN_DIRECTION_ENC) { return osmo_csn1_stream_encode(ar, CSNDESCR(Additional_access_technologies_t), vector, readIndex, data); } diff --git a/src/rlcmac/csn1_ts_44_060.c b/src/rlcmac/csn1_ts_44_060.c index e8c03df..e3270b7 100644 --- a/src/rlcmac/csn1_ts_44_060.c +++ b/src/rlcmac/csn1_ts_44_060.c @@ -183,7 +183,7 @@ /*< EGPRS Ack/Nack Description IE >*/ static gint16 Egprs_Ack_Nack_Desc_w_len_Dissector(csnStream_t* ar, struct bitvec *vector, unsigned *readIndex, void* data) { - if (ar->direction == 0) + if (ar->direction == CSN_DIRECTION_ENC) { return osmo_csn1_stream_encode(ar, CSNDESCR(EGPRS_AckNack_Desc_t), vector, readIndex, data); } @@ -394,7 +394,7 @@ #if 0 gint16 Receive_N_PDU_Number_list_Dissector(csnStream_t* ar, struct bitvec *vector, unsigned *readIndex, void* data) { - if (ar->direction == 0) + if (ar->direction == CSN_DIRECTION_ENC) { return osmo_csn1_stream_encode(ar, CSNDESCR(Receive_N_PDU_Number_t), vector, readIndex, data); } -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/39783?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I216d4d94f585dd4dcf9947324fdbdc786ab2b5c5 Gerrit-Change-Number: 39783 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

3 months, 3 weeks

1
0
0 0

[XS] Change in osmo-pcu[master]: checkpath: Exclude csn1 files

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-pcu/+/39782?usp=email ) Change subject: checkpath: Exclude csn1 files ...................................................................... checkpath: Exclude csn1 files CSN.1 parsing code is inherited/co-maintained with wireshark.git code, and used to have its own coding style, so avoid enforcing osmocom code style in there. Change-Id: Ib6e7d2dcc3d8f262092f0100e55cad397aa226f4 --- M .checkpatch.conf 1 file changed, 3 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/82/39782/1 diff --git a/.checkpatch.conf b/.checkpatch.conf index d808861..8194ffb 100644 --- a/.checkpatch.conf +++ b/.checkpatch.conf @@ -1 +1,4 @@ +--exclude ^src/csn1.c$ +--exclude ^src/csn1.h$ +--exclude ^src/csn1_(enc|dec).c$ --exclude ^src/gsm_rlcmac.c$ -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/39782?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ib6e7d2dcc3d8f262092f0100e55cad397aa226f4 Gerrit-Change-Number: 39782 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

3 months, 3 weeks

1
0
0 0

[S] Change in osmo-pcu[master]: encoding: Use defines to set PAYLOAD_TYPE

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/39777?usp=email ) Change subject: encoding: Use defines to set PAYLOAD_TYPE ...................................................................... encoding: Use defines to set PAYLOAD_TYPE Change-Id: I3d663cb91672fb383aeb72b223490fa615bdcdf0 --- M src/encoding.cpp M src/gsm_rlcmac.c M src/gsm_rlcmac.h 3 files changed, 10 insertions(+), 10 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/encoding.cpp b/src/encoding.cpp index 75b9a30..b10b9a3 100644 --- a/src/encoding.cpp +++ b/src/encoding.cpp @@ -602,7 +602,7 @@ Dynamic_Allocation_t *da; /* RLC/MAC control block without the optional RLC/MAC control header */ - block->PAYLOAD_TYPE = 0x01; // Payload Type + block->PAYLOAD_TYPE = PAYLOAD_TYPE_CTRL_NO_OPT_OCTET; // Payload Type block->RRBP = rrbp; // RRBP (e.g. N+13) block->SP = poll; // RRBP field is valid block->USF = 0x00; // Uplink state flag @@ -720,7 +720,7 @@ uint8_t tn; - block->PAYLOAD_TYPE = 0x1; // RLC/MAC control block that does not include the optional octets of the RLC/MAC control header + block->PAYLOAD_TYPE = PAYLOAD_TYPE_CTRL_NO_OPT_OCTET; // RLC/MAC control block that does not include the optional octets of the RLC/MAC control header block->RRBP = rrbp; // 0: N+13 block->SP = poll; // RRBP field is valid block->USF = 0x0; // Uplink state flag @@ -1759,7 +1759,7 @@ uint8_t container_idx, PNCDContainer_t *container) { - block->PAYLOAD_TYPE = 0x1; // RLC/MAC control block that does not include the optional octets of the RLC/MAC control header + block->PAYLOAD_TYPE = PAYLOAD_TYPE_CTRL_NO_OPT_OCTET; // RLC/MAC control block that does not include the optional octets of the RLC/MAC control header block->RRBP = 0; // 0: N+13 block->SP = 0; // RRBP field is not valid block->USF = 0x0; // Uplink state flag @@ -1784,7 +1784,7 @@ uint16_t arfcn, uint8_t bsic, uint8_t container_id) { - block->PAYLOAD_TYPE = 0x1; // RLC/MAC control block that does not include the optional octets of the RLC/MAC control header + block->PAYLOAD_TYPE = PAYLOAD_TYPE_CTRL_NO_OPT_OCTET; // RLC/MAC control block that does not include the optional octets of the RLC/MAC control header block->RRBP = rrbp; // RRBP (e.g. N+13) block->SP = poll; // RRBP field is valid? block->USF = 0x0; // Uplink state flag diff --git a/src/gsm_rlcmac.c b/src/gsm_rlcmac.c index 931c85c..79fda90 100644 --- a/src/gsm_rlcmac.c +++ b/src/gsm_rlcmac.c @@ -36,12 +36,6 @@ #include <arpa/inet.h> #include <gprs_debug.h> -/* Payload type as defined in TS 44.060 / 10.4.7 */ -#define PAYLOAD_TYPE_DATA 0 -#define PAYLOAD_TYPE_CTRL_NO_OPT_OCTET 1 -#define PAYLOAD_TYPE_CTRL_OPT_OCTET 2 -#define PAYLOAD_TYPE_RESERVED 3 - /* CSN1 structures */ /*(not all parts of CSN_DESCR structure are always initialized.)*/ static const diff --git a/src/gsm_rlcmac.h b/src/gsm_rlcmac.h index c43d418..d34024a 100644 --- a/src/gsm_rlcmac.h +++ b/src/gsm_rlcmac.h @@ -48,6 +48,12 @@ typedef guint8 N51_t; typedef guint8 N26_t; +/* Payload type as defined in TS 44.060 / 10.4.7 */ +#define PAYLOAD_TYPE_DATA 0 +#define PAYLOAD_TYPE_CTRL_NO_OPT_OCTET 1 +#define PAYLOAD_TYPE_CTRL_OPT_OCTET 2 +#define PAYLOAD_TYPE_RESERVED 3 + /* Starting Time IE as specified in 04.08 */ typedef struct { -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/39777?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I3d663cb91672fb383aeb72b223490fa615bdcdf0 Gerrit-Change-Number: 39777 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

3 months, 3 weeks

1
0
0 0

2025

2024

2023

2022

gerrit-log March 2025