<p style="white-space: pre-wrap; word-wrap: break-word;">thx, will apply...</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/20332">View Change</a></p><p>5 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/20332/1/src/gsm/gad.c">File src/gsm/gad.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/libosmocore/+/20332/1/src/gsm/gad.c@37">Patch Set #1, Line 37:</a> <code style="font-family:monospace,monospace">        OSMO_VALUE_STRING(OSMO_GAD_TYPE_ELL_POINT),</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">probably want to have this as "ELL_POINT", etc. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">that's the style you guys use, yes. I really much much prefer to have in the log output the exact actual names that things have in the source code, because it makes working with the source so much easier. If the names are shortened, it adds another indirection every single time of first looking up the "weird" names in the value_string[], and only then can I grep the sources.</p><p style="white-space: pre-wrap; word-wrap: break-word;">If someone else adds to pespin's opinion I'll change it (and not like it, but ok)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/20332/1/src/gsm/gad.c@55">Patch Set #1, Line 55:</a> <code style="font-family:monospace,monospace">uint32_t osmo_gad_enc_lat(int32_t lat_deg_1e6)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Missing documentation in all functions. I think it is really needed here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">ack</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/20332/1/src/gsm/gad.c@162">Patch Set #1, Line 162:</a> <code style="font-family:monospace,monospace">#ifdef GAD_FUTURE</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">explain this?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">i'll add a comment</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/20332/1/src/gsm/gad.c@250">Patch Set #1, Line 250:</a> <code style="font-family:monospace,monospace">  *gad = (struct osmo_gad){};</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">what's the point of memzeroing here?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">reproducability in gad_test.c, which allows a memcmp(sizeof(struct osmo_gad))</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmocore/+/20332/1/src/gsm/gad.c@293">Patch Set #1, Line 293:</a> <code style="font-family:monospace,monospace">         OSMO_STRBUF_PRINTF(sb, "Ellipsoid-point-with-uncertainty-circle{to-str-not-implemented}");</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">all of these look like they should mostly be calls to get the value from value_string....</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">ok, can add such, but there still needs to be a switch() for the diverse elements to be printed.<br>So there will be a switch() here, *plus* an array iteration lookup.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Below points should also get their individual elements printed at some point, just so far osmo-smlc is only composing an ell point unc circle type, so i skipped the monkey work for now.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/20332">change 20332</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/c/libosmocore/+/20332"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I7a9dd805a91b1ebb6353bde0cd169218acbf223c </div>
<div style="display:none"> Gerrit-Change-Number: 20332 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-CC: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 01 Oct 2020 13:04:52 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>