Modify

Opened 6 years ago

Last modified 2 years ago

#15751 new enhancement

[PATCH] oneway=reversible|alternating for highways

Reported by: naoliv Owned by: team
Priority: normal Milestone:
Component: Internal preset Version:
Keywords: oneway reversible alternating Cc: Klumbumbus

Description

With https://twitter.com/bhousel/status/948995008404729856 I guess that the usage of oneway=reversible|alternating¹ might increase.

¹ https://wiki.openstreetmap.org/wiki/Key:oneway#List_of_values

I am unsure if JOSM should also include them in the presets, but at least it should ignore the values when validating (so we won't see messages like Value 'alternating" for key 'oneway' not in presets)

JOSM:

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2018-01-08 02:45:59 +0100 (Mon, 08 Jan 2018)
Revision:13300
Build-Date:2018-01-08 01:49:06
URL:http://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (13300 pt_BR) Linux Debian GNU/Linux testing (buster)
Memory Usage: 1349 MB / 7168 MB (769 MB allocated, but free)
Java version: 9.0.1+11-Debian-1, Oracle Corporation, OpenJDK 64-Bit Server VM
Screen: :0.0 1600x900, :0.1 1280x1024
Maximum Screen Size: 1600x1024
Java package: openjdk-9-jre:amd64-9.0.1+11-1
Java ATK Wrapper package: libatk-wrapper-java:all-0.33.3-13
VM arguments: [--add-modules=java.activation,java.se.ee, -Dawt.useSystemAAFontSettings=on]
Dataset consistency test: No problems found

Attachments (7)

josm_15751.patch (400 bytes ) - added by skyper 3 years ago.
patch silencing warning about both values for now
15751_v1.patch (7.7 KB ) - added by reichg 3 years ago.
move oneway checks to combos that include reversbiel|alternating
15751_v2.patch (8.8 KB ) - added by reichg 3 years ago.
only problem I can't solve is having evenly spaced checkboxes on Motorway, Motorway_Link, Trunk, and Trunk_Link. There are 2 rows of checkboxes and the 2nd row is not spaced out the same as the 1st row.
15751_v3.patch (11.3 KB ) - added by reichg 3 years ago.
Bandaid for now. New ticket should be made for revamp of the defaultpresets.xml file.
15751_v4.patch (12.2 KB ) - added by reichg 3 years ago.
moved toll and oneway to Road Restrictions item and moved motorroad to Access Restrictions.
15751_v5.patch (13.6 KB ) - added by reichg 3 years ago.
moves motorroad back into motorway - primary_link
15751_v6.patch (14.2 KB ) - added by reichg 2 years ago.
removed spaces at the beginning/end of chunks, added oneway combo in the highway_yesno chunk.

Download all attachments as: .zip

Change History (44)

comment:1 by Don-vip, 6 years ago

Cc: Klumbumbus added

Definition looks clear enough for me. Can we render these ways as iD does?

comment:2 by Klumbumbus, 6 years ago

OK, but I think we need two way arrows too. Simply using our current oneway arrows would be a bit confusing.

comment:3 by Don-vip, 6 years ago

That's what I meant: is JOSM already capable of displaying two arrows, or do we need to implement it first?

in reply to:  3 comment:4 by Klumbumbus, 6 years ago

Replying to Don-vip:

is JOSM already capable of displaying two arrows?

I don't think so.

If you implement this maybe consider if it makes sense (and not too much work) to make them adjustable, so we could display oneway:bycycle=no arrows in a different style, similar to opencyclemap.

comment:5 by rcruz05@…, 6 years ago

Which tag can I change to have vehicles drive on the left hand side?

comment:6 by reichg, 3 years ago

looks like the alternating|reversible modeling is not supported still by josm. While that is true I am not seeing the warning Value 'alternating" for key 'oneway' not in presets as stated above when I use oneway=reversible|alternating? Has this been resolved?

by skyper, 3 years ago

Attachment: josm_15751.patch added

patch silencing warning about both values for now

comment:7 by skyper, 3 years ago

Nothing has changed but the common warning about values not in presets is an informal (other) warning and still present once these warnings are enabled in preferences.

I've attached a patch silencing validator for now. Should be removed again once these values enter defaultpresets.

In my eyes, the most interesting part are more flexible arrows like more than one and different/multiple colors. This is probably worth a new ticket.

comment:8 by skyper, 3 years ago

Keywords: oneway reversible alternating added
Summary: oneway=reversible|alternating for highways[Patch] oneway=reversible|alternating for highways

comment:9 by reichg, 3 years ago

Milestone: 21.08

comment:10 by Klumbumbus, 3 years ago

I think we should add the two values to the defaultpresets instead. Or is there something blocking it (except that we need to transform the checks into combos)?

comment:11 by skyper, 3 years ago

Component: Core validatorInternal preset
Milestone: 21.08
Summary: [Patch] oneway=reversible|alternating for highwaysoneway=reversible|alternating for highways

Besides medium to low numbers of use and only in use, e.g. no proposal, there is nothing wrong or disputing. Changing the checks to combos has some impact on the presets layout, though, especially, in places where grouped checks are used. So little work to do but shouldn't be that big problem

comment:12 by reichg, 3 years ago

I want to make sure I am understanding the defaultpresets.xml file properly:

for id="highway_base" is it not as simple as adding -> <combo key="oneway" text="Oneway" values="yes,no,reversible,alternating" values_sort="true" />

What is id="highway_base" referring to, a way with a highway tag present on it?

IS there any documentation on how the xml file works for presets?

comment:13 by skyper, 3 years ago

TaggingPresets

id="" is the name of the `<chunk>` which is used in several <item> or <chunk> with <reference ref="" />

<check /> can be grouped together with <checkgroup> so simply changing the <check /> into a <combo /> does not work in all places. Additionally, the layout and order of tags in the presets should be looked at.

in reply to:  13 comment:14 by reichg, 3 years ago

Replying to skyper:

TaggingPresets

id="" is the name of the `<chunk>` which is used in several <item> or <chunk> with <reference ref="" />

<check /> can be grouped together with <checkgroup> so simply changing the <check /> into a <combo /> does not work in all places. Additionally, the layout and order of tags in the presets should be looked at.

Thank you for the information! I will get on this!

comment:15 by reichg, 3 years ago

Am I correct that the oneway presets we are adding should be on anything with a highway tag?

answered this on my own. 99.14% of features with the oneway tag on it are on features with highway tag.

Last edited 3 years ago by reichg (previous) (diff)

by reichg, 3 years ago

Attachment: 15751_v1.patch added

move oneway checks to combos that include reversbiel|alternating

comment:16 by reichg, 3 years ago

15751_v1.patch includes all of the oneway check boxes converted to combos which include the desired values reversible|alternating.

Last edited 3 years ago by reichg (previous) (diff)

comment:17 by reichg, 3 years ago

Summary: oneway=reversible|alternating for highways[PATCH] oneway=reversible|alternating for highways

comment:18 by skyper, 3 years ago

The columns="" of the <checkgroup> need to be adjusted.

comment:19 by skyper, 3 years ago

Oh, forgot about the last enhancement of placement of <checkgroup>. So only some columns need adjustment but I need to take a closer look and fear that some more rearranging is useful to get a nice layout of consecutive checkgroups. Especially 'highway_yesno_incline_oneway_lit_width' does not look nice anymore.

in reply to:  19 comment:20 by reichg, 3 years ago

Replying to skyper:

Oh, forgot about the last enhancement of placement of <checkgroup>. So only some columns need adjustment but I need to take a closer look and fear that some more rearranging is useful to get a nice layout of consecutive checkgroups. Especially 'highway_yesno_incline_oneway_lit_width' does not look nice anymore.

I will check these and fix them!

by reichg, 3 years ago

Attachment: 15751_v2.patch added

only problem I can't solve is having evenly spaced checkboxes on Motorway, Motorway_Link, Trunk, and Trunk_Link. There are 2 rows of checkboxes and the 2nd row is not spaced out the same as the 1st row.

in reply to:  19 comment:21 by reichg, 3 years ago

Replying to skyper:

Oh, forgot about the last enhancement of placement of <checkgroup>. So only some columns need adjustment but I need to take a closer look and fear that some more rearranging is useful to get a nice layout of consecutive checkgroups. Especially 'highway_yesno_incline_oneway_lit_width' does not look nice anymore.

Patch 15751_v2 looks pretty good now with 5 columns for 'highway_yesno_incline_oneway_lit_width', just need to figure out the spacing as mentioned with the patch attachment:

only problem I can't solve is having evenly spaced checkboxes on Motorway, Motorway_Link, Trunk, and Trunk_Link. There are 2 rows of checkboxes and the 2nd row is not spaced out the same as the 1st row.

comment:22 by skyper, 3 years ago

The spacing problem can only be solved by using one checkgroup instead of several (or core needs to be enhanced).

I took a closer look and found a minor issue regarding oneway:bicyle which should be right below oneway.

In general, I would rework the chunks and checkgroups.

  • I believe lit could be added to "highway_yes/no" as even rails are sometimes lit and all highways can be for sure.
  • incline is also useful for all.
  • Regarding the spacing all checks should be in one checkgroup which means some more chunks are needed for e.g. motorroad and toll
    • Maybe, having chunks for only the checks and defining the checkgroups in another chunk together with the other tags or even in the item itself works better.
    • Another option might be moving all tags besides the checks into own chunks and either group them in another chunk or reference both chunks in the item.

by reichg, 3 years ago

Attachment: 15751_v3.patch added

Bandaid for now. New ticket should be made for revamp of the defaultpresets.xml file.

comment:23 by reichg, 3 years ago

So, patch 15751_v3 moved lit, toll, and motorroad to the "highway_yesno" chunk. This puts them in a lot of presets where they won't be used but for now it seems fine since toll and lit are useful in most scenarios. Motorroad can be moved around in the new ticket for a defaultpresets revamp?

I can create that ticket for a defaultpresets.xml file revamp.

Version 1, edited 3 years ago by reichg (previous) (next) (diff)

comment:24 by skyper, 3 years ago

  • toll is in "Road Restrictions" which is linked to with preset_link and might not be needed at all.
  • motorroad should only be present with useful combinations, e.g. own chunk.
  • there is no need for a checkgroup with only one check (oneway:bicycle).
  • Seven checks in a row might be to much, I'd rather use columns=4 and have the checks spread over two rows.
    • Five checks in a row should work, so this might be only interesting for groups with motorroad and/or toll.

comment:25 by reichg, 3 years ago

  • So remove toll from Road Restrictions?
  • Should motorroad have its own individual chunk with one check in it? Reference to a chunk with checks is not allowed within checkgroups so it would be a single check on one its own line.
  • removed checkgroups with only one check!
  • Easily can the columns!

comment:26 by skyper, 3 years ago

Sorry, my last comment was a bit miss-understandable.

  • I thought about removing toll from the primary presets and leave "Road Restriction" untouched. After a second look, I see, that oneway is present in both, too, but I did not check the opposite, if oneway is present in every primary preset linking to "Road Restrictions".
    • Atm, the same tag in a primary preset and a linked presets does not work, see #18992.
  • Only one checkgroup for all. motorroad should be in the chunk (checkgroup) for trunk[_link] and primary[_link].
    • primary[_link] has the check below in a second group which is irritating.

Checkgroup should work pretty well after the changes some month ago, even with multiple columns like nine checks with columns=4.
Some of the cleanup, like the same tag in the primary and the linked to preset, could be made in #21308 or #18992 might get fixed. In the end, I can life with little duplication as "Road Restriction" and "Access Restrictions" are useful with the links to presets from property/membership panel.

I am sorry for the amount of critics and some problems like the spacing and a strange vertical space (#21319) are present atm, too. These checkgroup chunks and the highway presets are quite complicated and not the easiest task for the first time modifying defaultpresets.xml.

Last edited 3 years ago by skyper (previous) (diff)

by reichg, 3 years ago

Attachment: 15751_v4.patch added

moved toll and oneway to Road Restrictions item and moved motorroad to Access Restrictions.

comment:27 by reichg, 3 years ago

No worries at all, our discussions are enjoyable and productive!

patch 15751_v4 moved toll/oneway to Road Restriction item from their original chunks. Also moved motorroad to Access Rstrictions since it historically has been used on Motorway through Secondary and not just the links. duplicates in linked preset should be eliminated.

Max amount of columns used is 4 because it looks good so far.

Last edited 3 years ago by reichg (previous) (diff)

in reply to:  27 ; comment:28 by skyper, 3 years ago

Replying to reichg:

No worries at all, our discussions are enjoyable and productive!

Nice, that you take it this way. I sometimes fear that I miss the point and that I am not clear enough.

patch 15751_v4 moved toll/oneway to Road Restriction item from their original chunks. Also moved motorroad to Access Restrictions since it historically has been used on Motorway through Secondary and not just the links. duplicates in linked preset should be eliminated.

+1 for adding motorroad to "Access Restrictions" but in my eyes, I would not remove that many tags from the primary presets, especially tags with high numbers as combination, which leads us to #21308.

I really hope that #18992 will get fixed soon as little duplication does not harm and makes it more flexible. Looking at external presets which extend other presets like Lane Attributes or Public Transport GTFS duplication exist as I do not want to rip some keys out of context once they enter defaultpresets.

in reply to:  28 comment:29 by reichg, 3 years ago

Replying to skyper:

Replying to reichg:

No worries at all, our discussions are enjoyable and productive!

Nice, that you take it this way. I sometimes fear that I miss the point and that I am not clear enough.

patch 15751_v4 moved toll/oneway to Road Restriction item from their original chunks. Also moved motorroad to Access Restrictions since it historically has been used on Motorway through Secondary and not just the links. duplicates in linked preset should be eliminated.

+1 for adding motorroad to "Access Restrictions" but in my eyes, I would not remove that many tags from the primary presets, especially tags with high numbers as combination, which leads us to #21308.

I really hope that #18992 will get fixed soon as little duplication does not harm and makes it more flexible. Looking at external presets which extend other presets like Lane Attributes or Public Transport GTFS duplication exist as I do not want to rip some keys out of context once they enter defaultpresets.

so can I just add the one off motorroad to the Motorway - Secondary items for now so we can close this ticket and move on to #21308? Wouldn't it be best to have all duplicates removed between primary and linked presets in the future for simple organization sake? I can work on the #21308.

comment:30 by skyper, 3 years ago

I am fine with that.

I do not think that we get rid of all duplicates. We want to emphasis on certain tags like motorroad=yes together with highway=trunk[_link]/primary[_link] but still want to have it for general use in "Access Restrictions". Similar with oneway=* which is important for highway=motorway_link and useful together with every highway link but not that important for path.

by reichg, 3 years ago

Attachment: 15751_v5.patch added

moves motorroad back into motorway - primary_link

comment:31 by reichg, 3 years ago

Just added motorroad where it probably looks best, added a space under the name/ref checkboxes.

comment:32 by reichg, 3 years ago

how does this look? Add milestone?

comment:33 by skyper, 3 years ago

  • Please, be careful with <space /> at the top or bottom of chunks. E.g. in many presets there is two much empty space below smoothness=*, now.
  • I still do not like that oneway=* is only present in the "Road Restriction" preset, now, but less important keys are in the main preset . Plus the maxspeed=* entry are still in both and somehow depend on oneway=* as backward/forward make seldomly sense with oneway=yes.
    • additionally oneway:bicycle is now lonely, again, but oneway=* should be right above. No objections to add oneway:bicyle=* to "Road Restrictions" even though it adds another duplication.
  • Can we stick to this ticket and add the values for oneway=* plus adjust the individual chunks but leave out the moving or removing of keys in the presets for another ticket?

by reichg, 2 years ago

Attachment: 15751_v6.patch added

removed spaces at the beginning/end of chunks, added oneway combo in the highway_yesno chunk.

comment:34 by reichg, 2 years ago

Let me know if anything else needs to be moved around or if I need to revert some of the other changes not applicable to this ticket.

comment:35 by skyper, 2 years ago

  • Oh, I just noticed that all default="" values are dropped. Regarding motorroad there should be no problem to keep it. Regarding oneway only one chunk for all highway classes does not work if we want to keep it.
  • I am not sure about motorroad for secondary highways and below. According to taginfo the combination has only low numbers ([1], [2]). I'd rather would add it to the Access Restriction preset and would life with another duplicate for primary highways and above.
  • Do we need the <space /> above motorroad? At least it should be consistent. Right now it is a mix.
Last edited 2 years ago by skyper (previous) (diff)

comment:36 by skyper, 2 years ago

Oh, one more: I would not change the order of the checks on top of "Road Restriction" preset. Categorical order is better than alphabetical one, especially, if you count all translation languages.

comment:37 by skyper, 2 years ago

@reichg:
Are you still working on this?
In #21396 oneway:moped=no is mentioned as next oneway tag which needs to be added, but how?

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from team to the specified user.
Next status will be 'needinfo'. The owner will be changed from team to naoliv.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.
The owner will be changed from team to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.