Opened 8 years ago

Last modified 6 years ago

#730 new enhancement/feature request

Tag maxspeed:forward / maxspeed:backward ignored [WITH FIX]

Reported by: bjoernchr74 Owned by: KaZeR
Priority: minor Milestone: version 0.6.0
Component: core Version: git master
Severity: Keywords: maxspeed:forward maxspeed:backward, routing, lanes, süeed
Cc:

Description

I've begun adding lots of speedlimits around my area and many of them require the above OSM tags - maybe a "german" thing, I don't know. But naturally, the "maxpsed" tag is never used in this scenario. Which leads to Navit showing speedlimits like "65 km/h" which I've never seen before ;)

Now I was going totally crazy as Navit never showed these properly on my Android. Now I finally understood why - "navit/maptool/osm.c" doesn't understand this one :) I actually changed the code in "osm_add_tag" to be "else if" and if the tag is not recognized give some output.

So I guess there are two issues here: 1) The tag should be "interpreted" by the maptool and the BIN file created accordingly 2) The tag should be "used" by Navit when it's running

Attachments (5)

attr_def.h.diff (346 bytes) - added by bjoernchr74 8 years ago.
osm.c.diff (5.5 KB) - added by bjoernchr74 8 years ago.
route.c.diff (9.9 KB) - added by bjoernchr74 8 years ago.
route.h.diff (573 bytes) - added by bjoernchr74 8 years ago.
track.c.diff (2.1 KB) - added by bjoernchr74 8 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 8 years ago by korrosa

  • Keywords maxspeed:forward maxspeed:backward added; maxspeed:forwards maxspeed:backwards removed
  • Type changed from defect/bug to enhancement/feature request

I don't have much opinion on this particular ticket, but thought I'd bring in some facts:

According to http://tagstat.hypercube.telascience.org/:

  • maxspeed - appears 1,462,348 times in the planet file
  • maxspeed:backwards - appears 2 times in the planet file
  • maxspeed:backward - appears 1,735 times in the planet file
  • maxspeed:forwards - appears 0 times in the planet file
  • maxspeed:forward - appears 1,766 times in the planet file

Neither maxspeed:forward nor maxspeed:backward appear in the main OSM wiki, but are mentioned in the German language version of the maxspeed page: http://wiki.openstreetmap.org/wiki/DE:Key:maxspeed

Opinion: I don't think that it is currently easy to assign two different speed limits to the same way within the Navit binfile.

comment:2 Changed 8 years ago by bjoernchr74

Yeah, I guess it's really a "german" thing. Here we sometimes have 5 kilometers of a speed limit in one direction but not the other (similar to "overtaking=forward" etc).

I'm looking at the navit code now and will play around with it a bit. If I come up with something I'll attach it here.

Would it be possible to however at least add a "workaround" in maptool/osm.c for the moment to use the "slowest defined value greater than 0"? E.g. in a case like this "Setting max speed 50 based on: 0 60 50" (parameters maxspeed, maxspeed:forward, maxspeed:backward)? That way, German users will at least have "some" indication of "some" maxspeed and will not assume the maximum... This should be quite trivial to add and won't "hurt" I guess :)

comment:3 Changed 8 years ago by korrosa

You'll want to look at line 923 within osm.c in the maptool folder: http://navit.svn.sourceforge.net/viewvc/navit/trunk/navit/navit/maptool/osm.c?view=markup

I guess you could do something like (in psuedo code):

if(tag="maxspeed:forwards"){
    maxspeed_value = value
}
if(tag="maxspeed:backwards"){
    if(!maxspeed_value || value < maxspeed_value){
        maxspeed_value = value
    }
}
if(tag="maxspeed"){
    maxspeed_value = value
}

This way, the "maxspeed" tag ALWAYS overwrites any "maxspeed:forward" or "maxspeed:backward" tags. If there is no "maxspeed" tag, then the lowest value of either "maxspeed:forward" or "maxspeed:backward" is used.

Actually, I'm just repeating what you said - I'm no c-coder, but it doesn't look to tricky to implement. Hopefully the line number is a start!

comment:4 Changed 8 years ago by korrosa

Maybe something like this:

if (! strcmp(k,"maxspeed:forward")) {
	if (strstr(v, "mph")) {
		maxspeed_attr_value = (int)floor(atof(v) * 1.609344);
	} else {
		maxspeed_attr_value = atoi(v);
	}
	if (maxspeed_attr_value)
		flags[0] |= AF_SPEED_LIMIT;
	level=5;
}
if (! strcmp(k,"maxspeed:backward")) {
	if(!maxspeed_attr_value || atoi(v) < maxspeed_attr_value){
		if (strstr(v, "mph")) {
			maxspeed_attr_value = (int)floor(atof(v) * 1.609344);
		} else {
			maxspeed_attr_value = atoi(v);
		}
		if (maxspeed_attr_value)
			flags[0] |= AF_SPEED_LIMIT;
		level=5;
	}
}
if (! strcmp(k,"maxspeed")) {
	if (strstr(v, "mph")) {
		maxspeed_attr_value = (int)floor(atof(v) * 1.609344);
	} else {
		maxspeed_attr_value = atoi(v);
	}
	if (maxspeed_attr_value)
		flags[0] |= AF_SPEED_LIMIT;
	level=5;
}

comment:5 Changed 8 years ago by bjoernchr74

Thanks korrosa :)

Well I fixed maptool in such a way that it works for me to check that the speed limits "are in OSM" but not displayed properly yet in Navit.

I also tried to fix OSD but that is a bit more tricky... haven't found out yet how I can properly determine if I'm going "forward" = "in the direction of the road" or not (checking "attr_directed" in "osd_text_draw()" apparently doesn't work). I guess I'll check on IRC if someone has an idea.

Changed 8 years ago by bjoernchr74

Changed 8 years ago by bjoernchr74

Changed 8 years ago by bjoernchr74

Changed 8 years ago by bjoernchr74

Changed 8 years ago by bjoernchr74

comment:6 Changed 8 years ago by bjoernchr74

I completed it. I used "diff -c" to create the diff file, works fine when I use "patch --dry-run -p1 -i ..."

Basically, we need to create a new maptool which will fill the navitmap.bin with the data.

Afterwards, the navit code can "figure out" what is forward/backward on a given street and will maintain the speeds properly. The OSD will also display them correctly.

Took me quite a while to figure this out, but it works fine for me :) Hope this can be checked in - there are other things I want to look at as well (but not such dramatic changes)

comment:7 Changed 8 years ago by korrosa

  • Summary changed from Tag maxspeed:forward / maxspeed:backward ignored to Tag maxspeed:forward / maxspeed:backward ignored [WITH FIX]

A thought: if this new maptool was deployed on the http://maps.navit-project.org/ and people with older versions of Navit started to use them, how would this affect Navit's performance? i.e. are the maps backwards compatible?

comment:8 Changed 8 years ago by bjoernchr74

Yes, a new map will work with an older Navit release. I have the new navitmap.bin

  • on my Linux System (with my new code) and
  • on my Android where I have a one week old Navit release (as I can't get the cross-compiler build to work)

The map works fine, I use it in my car. There is no performance penalty as the routing code ignores these new speeds currently.

comment:9 Changed 8 years ago by bjoernchr74

And an old map also works with the new code, just tried.

But please verify it before it goes into the SDK :)

comment:10 Changed 8 years ago by bjoernchr74

Sorry for the "spamming"... the diff/patch didn't work as I expected (I never used that before to be honest). "patch" states it did the changes, but none are there... So I replaced the files (knowing that the only diff are my changes so it's fine) and it works properly then with "fresh" SDK code and my replaced files. If you want me to do the "proper" diff/patch or even with SVN please let me know how and I'll do that.

comment:11 Changed 8 years ago by bjoernchr74

Just wondering if someone wants to test this and maybe check it in if it's fine? I thought it might happen a bit faster ;) The changes look like a lot but they are really not that much...

I have some other stuff I'd like to fix as well but I don't want to have my code run out of sync here too much... Thanks!

comment:12 Changed 8 years ago by tegzed

Hi Bjoernchr74,

I haven't tested your patch, just run through the diffs. I have an observation about attribute position in attr_def.h. Please put new attributes to the end of the section for a specific type of attribute. Not doing so will shift the attribute type's numeric values after the inserted one in the section. This will make new maps incompatible with previous navit versions. I'll try to find some time to check your patch however the best wouold be to get the blessing of Cp15 to this patch before committing.

comment:13 Changed 6 years ago by usul

  • Keywords routing lanes added
  • Priority changed from major to minor

Any opinions about lane specific routings?

comment:14 Changed 6 years ago by usul

  • Keywords süeed added
  • Milestone set to version 0.6.0

As not related to stability, I will just schedule it for next major release

Note: See TracTickets for help on using tickets.