[PATCH 1/2] Add convolutional code generators for CS2/3

Max msuraev at sysmocom.de
Wed Apr 20 07:58:24 UTC 2016


Thanks for review. Comments are inline.

On 04/20/2016 07:13 AM, Sylvain Munaut wrote:
> Hi,
>
>
> A few more comments, although those are mainly style so unless someone
> else also has those concerns, I'm not opposed to merging.
>
>
>>  src/gsm/conv_cs2_gen.c        | 211 +++++++++++++++++++++++++++++
>>  src/gsm/conv_cs3_gen.c        | 299 ++++++++++++++++++++++++++++++++++++++++++
> Why the _gen suffix ?
>
> If they're fully automatically generated, then the build process
> should generate them, they should not be in the repo at all.

That's actually the thing we should discuss: the latest version is more
or less complete - it can generate everything necessary, to use results
we only need one-liner "extern osmo_conv...." in the header. But if we
integrate it into build without adding generated files into repo than
we'll have hard build dependency on python. That's fine by me - I don't
think we support any platform which does not have python. But I'd like
to hear what others think.

>
>> +extern const struct osmo_conv_code osmo_conv_gsm0503_cs2;
>> +extern const struct osmo_conv_code osmo_conv_gsm0503_cs3;
> Looking at all the other symbol in osmogsm, it seems we deemed 'osmo_'
> to be redudant and so maybe "gsm0503_conv_cs2" would fit better in the
> naming scheme ?
>
>
>> +static const uint8_t osmo_conv_gsm0503_cs2_output[][2] = {
> All those being purely internal symbols, then don't need osmo_conv either

Both are fine by me and are trivial to fix. Thanks.

>
> Cheers,
>
>    Sylvain

-- 
Max Suraev <msuraev at sysmocom.de> http://www.sysmocom.de/
======================================================================= 
* sysmocom - systems for mobile communications GmbH
* Alt-Moabit 93 
* 10559 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B 
* Geschaeftsfuehrer / Managing Directors: Holger Freyther, Harald Welte 



More information about the OpenBSC mailing list