[osmo-bts] Review of jolly/trx jolly/l1sap

Hi Andreas, I've found some time to review the trx and l1sap branches you have created in the osmo-bts.git repository. Let me start by mentioning that I support and encourage your work. However, I think we have to set some general rules before we can merge the code. For us, the most important factor is that we do not loose features or introduce regressions with more or less every new commit to osmo-bts. Furthermore, we would like to stay with 'one commit, one feature/fix'. I understand your primary goal ist go generalize some code and then create a working solution for the OpenBTS or OsmocomBB based transceiver. However, for the way to get the changes are integrated is to first introduce all the infrastructure/core changes while keeping osmo-bts-sysmo functional and that with all existing features. Once those core/infrastructure changes are merged, we can easily include support the extra code for the osmo-bts-trx and in fact even do all development commits to master, as long as they don't touch the core code. Looking at your commits: http://cgit.osmocom.org/cgit/osmo-bts/commit/?h=jolly/trx&id=a829957383… is fine, as it just removes old code. http://cgit.osmocom.org/cgit/osmo-bts/commit/?h=jolly/trx&id=9be2099855… looks fine to me, too, we should be able to merge that. My only comment would be: Are you sure this should not be configured via OML from the BSC? I think TS 12.21 Chapter 9.4.14 should be the proper way to configure it. Do you agree? If yes, please use OML configuration and remove the VTY option. http://cgit.osmocom.org/cgit/osmo-bts/commit/?h=jolly/trx&id=7322254e68… seems fine to me. Can you pleaes confirm from your side that it _should_ not alter/remove any existing code/behavior, but just introduce that L1SAP intermediate interface? Have you tested it with osmo-bts-sysmo? http://cgit.osmocom.org/cgit/osmo-bts/commit/?h=jolly/trx&id=1c37129621… seems to not just remove the direct handling of primitives, but also add some code, particularly the gsmtap_ip command line argument. Can you please split that in a separate patch? Also, have you tested osmo-bts-sysmo after this commit? Regards, Harald -- - Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)