This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Vadim Yanitskiy gerrit-no-reply at lists.osmocom.org
Patch Set 4:
(15 comments)
https://gerrit.osmocom.org/#/c/816/4//COMMIT_MSG
Commit Message:
Line 12: beeter to clean up the OsmoBTS source code, sharing this code.
> Better than what?
Yes, thanks. I'll change it.
Line 28: in generated state.
> The general opinion about generated codes was to rather not
In OsmoBTS there are already generated convolutional codes. So, I spent some time finding corresponding polynomial sets for some data types, which wasn't defined in the conv_gen.py generator.
Despite the EDGE's MCS levels use the same generator polynomial with different lengths, I wasn't able to find any details about EDGE convolutional transcoding, so I merely copied those tables from OsmoBTS into a separate file.
I still have some doubts, related to MCS codes...
They aren't covered by GSM 05.03, so should I migrate them in separate commit? Should I use a separate file? Which name to use for this file?
https://gerrit.osmocom.org/#/c/816/4/include/osmocom/gsm/gsm0503_coding.h
File include/osmocom/gsm/gsm0503_coding.h:
Line 6: * (C) 2016 by Tom Tsou <tom.tsou at ettus.com>
> we usually have the copyright and license info only in the .c files...
I just followed the gsm0503.h as an example, because there was such information... But I am agree with you.
Line 46: int gsm0503_xcch_encode(ubit_t *bursts, uint8_t *l2_data);
> you're using stdint.h types, yet this header file has no #includes at all.
Ok, I need to add the following:
#include <stdint.h>
#include <osmocom/core/bits.h>
https://gerrit.osmocom.org/#/c/816/4/src/gsm/Makefile.am
File src/gsm/Makefile.am:
Line 21: gsm610.c gsm620.c gsm660.c gsm0503_conv.c \
> Is the gsm610.c in src/codec related to these?
This is because the gsm0503_coding.c has some dependences from libosmocodec. Maybe there is a better way to link these dependences?
Line 30:
> don't add a blank line here:
Ok, I will fix it.
Line 42: $(AM_V_GEN)python2 ../../utils/conv_gen.py
> for 'make distcheck': rather use $(top_srcdir)/... ?
Ok, it would be better.
https://gerrit.osmocom.org/#/c/816/4/src/gsm/gsm0503_conv_edge.c
File src/gsm/gsm0503_conv_edge.c:
Line 62:
> one blank line too many -- but if the file is moving from
Yes, it is a code part from OsmoBTS, and there was even more blank lines, twice after every 'const struct osmo_conv_code...' definition. :) I used this separation (two blank lines) to separate tables from definitions and to increase readability...
https://gerrit.osmocom.org/#/c/816/4/src/gsm/gsm0503_interleaving.c
File src/gsm/gsm0503_interleaving.c:
Line 304:
> (two blank lines, same as before)
Just copied 'as is', but I can fix it.
https://gerrit.osmocom.org/#/c/816/4/src/gsm/gsm0503_mapping.c
File src/gsm/gsm0503_mapping.c:
Line 29: sbit_t *hl, sbit_t *hn)
> this file mixes single-tab indent and aligned indent styles.
Ok, will be fixed.
https://gerrit.osmocom.org/#/c/816/4/src/gsm/gsm0503_parity.c
File src/gsm/gsm0503_parity.c:
Line 33:
> would be nicer to not have a blank line between comment-for-struct and stru
It also was taken 'as is', will be fixed.
https://gerrit.osmocom.org/#/c/816/4/utils/conv_gen.py
File utils/conv_gen.py:
Line 30: name = "call-me", description = False, puncture = []):
> IMHO pretty weird indenting choice.
How to separate this big line better? I have some doubts... How many tabs should I use?
Line 53: raise ValueError("Bad polynoms: "
> The proper English term would be polynomial / polynomials.
I am not an author of this code, but I will fix it.
Line 59: "in a recursive code")
> separate commit.
Ok, I was thinking about that...
Line 298: 15, 19, 23, 27, 31, 35, 43, 47, 51, 55, 59, 63, 67, 71,
> I hope this is correct :)
For me this is magic too, and this is why I merely copied the puncture tables 'as is'. Just decreased tabs count...
--
To view, visit https://gerrit.osmocom.org/816
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I257a5d015798ee9e690fd035ca97fd971cf9f60a
Gerrit-PatchSet: 4
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-HasComments: Yes