On Sat, Oct 16, 2010 at 8:42 AM, Frederik Ramm <span dir="ltr"><<a href="mailto:frederik@remote.org">frederik@remote.org</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi,<div class="im"><br>
<br>
On 10/16/2010 03:25 PM, Stefan de Konink wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also, can you give me the URL for the source code for pbf2osm to check<br>
that a few edge-cases are correctly handled?<br>
</blockquote>
<br>
<a href="http://git.openstreetmap.nl/index.cgi/pbf2osm.git/" target="_blank">http://git.openstreetmap.nl/index.cgi/pbf2osm.git/</a><br>
</blockquote>
<br></div>
I'm hoping to take that, fix it up a bit (so that the resulting binary is not called "test" any more etc ;) and put in in the OSM SVN together with a set of Debian scripts that automatically build the protobuf stuff and produce a statically linked Debian package with little no dependencies. I think that should further reduce the hurdle to using it.<br>
</blockquote><div><br></div><div>Looks pretty good. I liked the coding style. Very simple, short, and concise. I have a few things to note.</div><div><br></div><div>When parsing the header:</div><div><br></div><div><div>    if (state == osmheader) {</div>
<div>        HeaderBlock *hmsg = header_block__unpack (NULL, bmsg->raw_size, uncompressed);</div><div>        if (hmsg == NULL) {</div><div>            fprintf(stderr, "Error unpacking HeaderBlock message\n");</div>
<div>            return 1;</div><div>        }</div><div><br></div><div>        if (verbose) fprintf(stderr, "%s\n", hmsg->required_features[0]);</div><div><br></div><div>        header_block__free_unpacked (hmsg, &protobuf_c_system_allocator);</div>
</div><div><br></div><div>The full array of required features should be checked and your program should error out if any required feature other than "OsmSchema-V0.6" or "DenseNodes" appears.</div><div>
<br></div><div>When parsing timestamps:</div><div><br></div><div><div>#define printtimestamp(attribute, timestamp) \</div><div>    char tsbuf[21]; \</div><div>    deltatime2timestamp(timestamp * (pmsg->date_granularity / 1000), tsbuf); \</div>
<div>    fputs_unlocked(" "attribute"=\"", stdout); \</div><div>    fputs_unlocked(tsbuf, stdout); \</div><div>    fputc_unlocked('"', stdout);</div></div><div><br></div><div>The formula is:</div>
<div>    (timestamp*date_granularity) / 1000</div><div><br></div><div>In the comments, you mention that the Nodes stuff is untested. You can build a file that uses Nodes instead of DenseNodes by invoking osmosis with '--write-pbf file=foo.osm.pbf usedense=false'.</div>
<div><br></div><div>Thanks, I enjoyed looking at your code and style of coding.</div><div>Scott</div><div><br></div></div>