Attention is currently required from: Hoernchen, Timur Davydov, fixeria, laforge.

pespin has posted comments on this change by Timur Davydov. ( https://gerrit.osmocom.org/c/osmo-trx/+/42411?usp=email )

Change subject: transceiver: add optional WebSDR device support ......................................................................

Patch Set 10:

(3 comments)

Patchset:

PS10: I'll try to summarize my thoughts, they may sound harsh but I hope you get the point in order to understand needed improvements: You are doing way too many changes in an invasive way, as in going with an axe disabling code and changing lots of code paths without properly modularizing code. This imho needs more work before it is in a point where it can be merged.

What this needs imho: * Properly split each change in code paths, etc. in little steps (commits), explaining why that change is needed. For instance, it makes no sense to have all the Transceiver code path method changes together with the new library/binary you are adding. This way we can also discuss each of the thing separately. * Properly split different code paths into different logical code blocks, eg. split read/write to socket into its own helpers if you don't need that, and then switch between those existing helpers and yours in an easy way.

So please, start by splitting out existing code paths in separate commits and explain why they need to be changed and how they improve the existing code.

File Transceiver52M/Transceiver.cpp:

https://gerrit.osmocom.org/c/osmo-trx/+/42411/comment/624f2983_3697441c?usp=... : PS10, Line 1132: msgLen = read(mDataSockets[chan], buffer, sizeof(buffer)); You should instead properly split read/write from/to sockets into its own method.

File Transceiver52M/radioInterface.cpp:

https://gerrit.osmocom.org/c/osmo-trx/+/42411/comment/e00dd06a_872a8224?usp=... : PS10, Line 332: int RadioInterface::fillPullBuffer(std::vector<short *> &bufs, size_t samples, int recvunderrun, TIMESTAMP ts) I don't see TIMESTAMP ts used here?