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=email : 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=email : 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? -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/42411?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ia0d340c323c2eea28fbe82601ba0af7cfbd68f6d Gerrit-Change-Number: 42411 Gerrit-PatchSet: 10 Gerrit-Owner: Timur Davydov <[email protected]> Gerrit-Reviewer: Hoernchen <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-CC: pespin <[email protected]> Gerrit-Attention: Hoernchen <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Attention: Timur Davydov <[email protected]> Gerrit-Comment-Date: Thu, 19 Mar 2026 07:57:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No
