[mkgmap-dev] [PATCH v8] - alpha patch to support road find by name

Mon Mar 23 08:57:24 GMT 2009

Hi Marko,

> On Sun, Mar 22, 2009 at 11:16:57PM +0000, Mark Burton wrote:
> > 
> > v9 - based on r985. Now tries to reduce (connected) groups of roads with
> > the same name and city to just one entry - please test.
> 
> Works for me.

Good.

> Some further observations about the incremental street name search.
> When I search for a road with ref=1521, as I type "1", the list will
> display both the road with ref=1 and road names starting with "1".
> (In Finland, street numbers come after the road name.  Thus, a street
> name can start with a number.)  When I type "15", I only see "15" on
> the list.  Likewise for "152" and "1521".  (All these roads exist.  If
> I type a digit after 1521, then there won't be matches.)  Also, when
> I type "E 7", the list only displays "7" (without the E).  With "E 75",
> it will display "E 75" as the only entry.
> 
> I'm beginning to think that Garmin has implemented it like this on purpose.

I think it's just plain broken!

> I didn't try adding any post code (zip) data yet.
> 
> I have some remarks on the patch itself.
> 
> > @@ -71,6 +71,7 @@
> >      <javac srcdir="${src}" destdir="${build.classes}" target="1.5" debug="true">
> >        <include name="**/*.java" />
> >        <classpath refid="main"/>
> > +      <!-- <compilerarg value="-Xlint:unchecked"/> -->
> >      </javac>
> >    </target>
> 
> Are you planning to uncomment this and fix the warnings?

I was trying to remove the warnings in Sortable but have not yet
managed to remove them all. I don't care too much (as it works as
expected) and there are quite a few other unchecked warnings, so I
shall probably not spend much time on that.

> 
> > +    	public int compareTo(Object other) {
> > +		Label o = (Label)other;
> > +		if(this == other)
> > +			return 0;
> > +		for(int i = 0; i < length && i < o.length; ++i) {
> > +			int diff = (ctext[i] & 0xff) - (o.ctext[i] & 0xff);
> > +			if(diff != 0)
> > +				return diff;
> > +		}
> > +		if(length == o.length)
> > +			return 0;
> > +		return (length > o.length)? 1 : -1;
> > +	}
> 
> Have you tested with non-ASCII data that this is what Garmin expects?

No.

> Which character set is ctext[] in?  What about the Sortable<String, ?>
> introduced in the patch?  Is the String in Unicode as usual?  Will the
> collation be compatible with Garmin's?  Will it depend on the locale setting?

Haven't a clue about any of that. But when people around the world test
this patch and report back, we will know whether it's doing the right
thing or not. I am ready to act on reports of it not working as
expected.

> 
> > @@ -41,10 +40,23 @@ public class Region {
> >  	}
> >  
> >  	public char getIndex() {
> > +		assert index > 0 : "Index not yet set";
> >  		return index;
> >  	}
> >  
> > +	public Country getCountry() {
> > +		return country;
> > +	}
> > +
> > +	public void setIndex(int index) {
> > +		this.index = (char)index;
> > +	}
> > +
> 
> Should setIndex assert that the index must not be set twice?

The answer to that depends on how paranoid you are. I can cope with
there not being an assert there because I know how that method is being
invoked.

Cheers,

Mark