<p style="white-space: pre-wrap; word-wrap: break-word;">I must admit that I see the general idea, which looks good, but I don't fully grok the fixme-aspects. So I just found a few typos while trying to figure those out... not important.</p><p><a href="https://gerrit.osmocom.org/11060">View Change</a></p><p>9 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11060/8//COMMIT_MSG">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11060/8//COMMIT_MSG@15">Patch Set #8, Line 15:</a> <code style="font-family:monospace,monospace">in the ASSIGNMENT COMPLETE speech codec (choosen) field are hardcoded.</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">"chosen" (yes, "to choose", but "chosen")</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c">File src/osmo-bsc/codec_pref.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@177">Patch Set #8, Line 177:</a> <code style="font-family:monospace,monospace">/* Helper function to generate the bss supported amr configuration bits (S0-S15) */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(I prefer not writing "Helper function" everywhere, all functions except main() could be seen as "helpers"; just say what the function does.)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@185">Patch Set #8, Line 185:</a> <code style="font-family:monospace,monospace"> /* Lookup the BTS specific AMR rate configuration. Thsi config is set</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">"This"</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@286">Patch Set #8, Line 286:</a> <code style="font-family:monospace,monospace"> {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(unusual position for '{')</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@288">Patch Set #8, Line 288:</a> <code style="font-family:monospace,monospace"> * configuration that we did not previously advertised as</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">"did .. advertise"</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/codec_pref.c@290">Patch Set #8, Line 290:</a> <code style="font-family:monospace,monospace"> * configuration enters the further processing steps we agein</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">"again"</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/lchan_fsm.c">File src/osmo-bsc/lchan_fsm.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/lchan_fsm.c@435">Patch Set #8, Line 435:</a> <code style="font-family:monospace,monospace"> /* FIXME: At the moment we can not support multiple codc rates in one</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">"codec"</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/lchan_fsm.c@437">Patch Set #8, Line 437:</a> <code style="font-family:monospace,monospace"> * for Threshold and Hysteresis. Those fiels are not needed when only</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">"fields"</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11060/8/src/osmo-bsc/lchan_fsm.c@440">Patch Set #8, Line 440:</a> <code style="font-family:monospace,monospace"> * struct would then also be different because each rate neads its</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">"needs"</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/11060">change 11060</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/11060"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I2d8ded51b3eb4c003fe2da6f2d6f48d001b73737 </div>
<div style="display:none"> Gerrit-Change-Number: 11060 </div>
<div style="display:none"> Gerrit-PatchSet: 8 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Assignee: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-CC: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 02 Oct 2018 13:56:34 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>