Attention is currently required from: jolly, pespin.

fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmocom-bb/+/34479?usp=email )

Change subject: ASCI: Add a flag to turn transmitter off or on
......................................................................


Patch Set 5: Code-Review-1

(2 comments)

Patchset:

PS5:
Looks good to me in general, however the L1CTL backwards compatibility needs to 
be kept IMO.


File include/l1ctl_proto.h:

https://gerrit.osmocom.org/c/osmocom-bb/+/34479/comment/f26a6b1b_89c286d9
PS5, Line 217:  uint8_t tch_flags;
> this means we probably need to update the L1CTL implementations everywhere, 
> eg TTCN3. […]
Indeed, adding a new field in the middle of the message is not a good idea 
because it would create compatibility issues. Adding new fields at the end is 
safer: the peer speaking an older L1CTL version would simply ignore them.

Also, I think I already proposed it during the Jitsi call, we could just re-use 
spare bits of the existing `audio_mode` field? Currently only 4 out of 8 are 
used by `AUDIO_{RX,TX}_*`. We could even rename it to `tch_flags`.



--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34479?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: I20133523adc3b204cd2181bfe664fe66020a10e3
Gerrit-Change-Number: 34479
Gerrit-PatchSet: 5
Gerrit-Owner: jolly <andr...@eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-CC: pespin <pes...@sysmocom.de>
Gerrit-Attention: jolly <andr...@eversberg.eu>
Gerrit-Attention: pespin <pes...@sysmocom.de>
Gerrit-Comment-Date: Thu, 21 Sep 2023 21:49:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: pespin <pes...@sysmocom.de>
Gerrit-MessageType: comment

Reply via email to