> The term "highwayCount" is inaccurate and the count is unimportant
> the only important concept is if a point on a line is a "node" or not.
>
> The term "node" is used in lots of different contexts of course, so
> what I mean in this context is:
>
> 1. a point that is used in more than one line is a "node"
 Ah, so you think that it doesn't matter whether the line is a road or not?

> 2. a boundary node is a "node" - it is joined to a line in another
> tile.
Again: All lines or just roads?

> 3. house numbers are tied to "node"s so you can have a "node" in the
> middle of a line, just to support a house number range.
>
> I used to say "routing node", but case (3) which I just discovered
> recently kind of makes that name incorrect too.
Ok, I think this is not yet handled when reading OSM data.

>
> > My understanding is this:
> > 1) highwayCount should be increased for each coord instance that is part of
> > a road.
>
> Not necessarily, it is just an implementation, a one bit counter would
> be enough.
>
> > I think the right place to do that is in an extra loop at the beginning of
> > StyledConverter.removeShortArcsByMergingNodes().
> > A coord instance that is part of more than one road is considered to become
> > a road junction.
>
> Its not just actual road junctions, two lines that join end to end
> as part of a single road in real life are still connected by a "node".
okay, understood.

>
> > 2) in many places I see something like this:
> > if(co.getOnBoundary()) {
> > // this point lies on a boundary
> > // make sure it becomes a node
> > co.incHighwayCount();
> > }
> > I think this is an abuse of the highwayCount. The highwayCount is
> > used to determine junctions of roads (e.g. in the frigRoundabout() method)
> > and I don't see why the attribute "onBoundary" should change the
> > "highwayCount"
>
> Agreed that the name is not appropriate and is confusing in this case.
> But the functionality is correct, a boundary node should be a "node".
>
> > If "onBoundary = true" and "highwayCount > 1" both have a meaning like
> > "isRoadJunction"
>
> isRoadJunction() is too narrow a name, all road junctions are nodes
> but not all nodes are road junction. But perhaps isNode()
Well, Node.isNode() looks not much better. May isGarminNode() ?

>
> > I suggest that we create a new flag with this name.
> >
> > Does that make sense?
>
> I'm fine with changing to better names + better comments so it
> doesn't cause confusion. Not so sure about algorithm changes that
> produce different results until you have identified a specific bug.
I am not sure if it is a bug. Many roads share the OSM nodes with e.g.
shapes or railway lines or something else. With the current implementation
these OSM nodes have highwayCount > 1 and therefor a CoordNode is
created for them. If this is intended then we don't have to change something,
else we could create smaller img files (e.g. 11.9 MB instead of 12.4 MB)

>
> > If yes, how can I verify the results of my changes ?
> > With my logic, I see ~20% fewer coords with highwayCount > 1^, so
> > the change is big.
>
> > The unit tests do not help much, SimpleRouteTest failes because
> > the NOD size is different, but I doubt that the expected size is a must?
>
> Correct, the size checks are just there to prevent accidental
> breakage, if you are fixing a bug that is expected to change the
> size then the test has to be changed too after very careful checking
> that the results are correct.
OK

>
> We don't have many good ways of checking results. The ultimate check
> is of course is testing how well routing works on an actual device (or
> range of devices, since they are all a bit different).
OK, up to now I did only check in BaseCamp, and I did probably not
check special cases like boundary nodes, CoordPOIs and so on.

>
> All there is at the moment is NetDisplay and NodDisplay in the display
> repo. We need better tools to display things are a higher level and
> automatically cross check between the different sections.
Yes, that would be great, but I still don't know enough to help here.

Ciao,
Gerd