Attention is currently required from: fixeria, osmith, pespin.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35123?usp=email )
Change subject: Add a GSM RLP decoder and encoder ......................................................................
Patch Set 7:
(5 comments)
File include/osmocom/gsm/rlp.h:
https://gerrit.osmocom.org/c/libosmocore/+/35123/comment/00ac2ee0_727e5ead PS4, Line 1: /*
maybe add a line description here pointing to 3GPP TS 24.022.
Done
https://gerrit.osmocom.org/c/libosmocore/+/35123/comment/a05a35af_4a0e4ba0 PS4, Line 24: /*! \defgroup rlp GSM RLP (Radio Link Protocol) as used in CSD
or add reference to 3GPP TS 24.022 here.
Done
File src/gsm/rlp.c:
https://gerrit.osmocom.org/c/libosmocore/+/35123/comment/c0ed4035_e0af9efa PS4, Line 85: //out->fcs = (data[240/8-3] << 16) | (data[240/8-2]) << 8 | (data[240/8-1] << 0);
this line can be dropped?
Done
https://gerrit.osmocom.org/c/libosmocore/+/35123/comment/9eb43577_a1c9b574 PS4, Line 89: if (n_s == 0x3f) {
switch (n_s)
Done
https://gerrit.osmocom.org/c/libosmocore/+/35123/comment/ea8475af_4940224c PS4, Line 93: memcpy(out->info, data+2, 240/8 - 5);
This 240/8 appears a lot of times, it may be worth describing it in a define.
I actually find it easier to understand the way it is. Anyone working on RLP will know the 240 / 576 byte frame size.
#define 240BITS_AS_BYTES (240/8) doesn't really make the code more readable, IMHO.
-
laforge