Modify

Opened 4 months ago

Last modified 4 months ago

#21163 new defect

Validating an object (way with conditionals) fails

Reported by: DodalerAfenger Owned by: team
Priority: normal Milestone:
Component: Core validator Version: tested
Keywords: template_report conditional highway lanes Cc:

Description

What steps will reproduce the problem?

  1. Validating this way

I came across that way by trying to solve an OSMOSE warning...

What is the expected result?

Looking at this page, I think the conditionals look ok. (Though I am not that much an expert ;-))

What happens instead?

Warnings are issued in JOSM saying the conditional keys use a wrong syntax on their values.

Please provide any additional information below. Attach a screenshot if possible.

URL:https://josm.openstreetmap.de/svn/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-07-12 02:41:41 +0200 (Mon, 12 Jul 2021)
Build-Date:2021-07-12 00:42:49
Revision:18004
Relative:URL: ^/trunk

Identification: JOSM/1.5 (18004 de) Windows 10 64-Bit
OS Build number: Windows 10 Pro 2009 (19042)
Memory Usage: 643 MB / 3616 MB (382 MB allocated, but free)
Java version: 1.8.0_301-b09, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920×1080 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: Cp1252
System property sun.jnu.encoding: Cp1252
Locale info: de_DE
Numbers with default locale: 1234567890 -> 1234567890
Dataset consistency test: No problems found

Plugins:
+ reverter (35732)

Last errors/warnings:
- 00005.719 W: Unable to request certificate of https://roottest-g3.pkioverheid.nl
- 00006.245 W: Unable to request certificate of https://roottest-g3.pkioverheid.nl
- 07173.462 W: No command found in undo/redo history, skipping confirmOrUndoMovement
- 07186.477 W: No command found in undo/redo history, skipping confirmOrUndoMovement
- 07501.212 W: java.net.SocketTimeoutException: Read timed out. Ursache: java.net.SocketTimeoutException: Read timed out
- 07501.213 E: java.net.SocketTimeoutException: Read timed out. Ursache: java.net.SocketTimeoutException: Read timed out
- 07501.229 E: org.openstreetmap.josm.io.OsmTransferException: Verbindung zum OSM-Server fehlgeschlagen. Bitte überprüfen Sie Ihre Internetverbindung.. Ursache: java.net.SocketTimeoutException: Read timed out. Ursache: java.net.SocketTimeoutException: Read timed out
- 07501.229 E: org.openstreetmap.josm.io.OsmTransferException: Verbindung zum OSM-Server fehlgeschlagen. Bitte überprüfen Sie Ihre Internetverbindung.. Ursache: java.net.SocketTimeoutException: Read timed out. Ursache: java.net.SocketTimeoutException: Read timed out
- 07501.229 E: org.openstreetmap.josm.io.OsmTransferException: Verbindung zum OSM-Server fehlgeschlagen. Bitte überprüfen Sie Ihre Internetverbindung.. Ursache: java.net.SocketTimeoutException: Read timed out. Ursache: java.net.SocketTimeoutException: Read timed out
- 07501.243 E: Ein-/Ausgabefehler - <html>Datenübertragungsfehler zum Server<br>"https://api.openstreetmap.org/api/0.6/map?bbox=9.1482263,48.7679374,9.1522263,48.7719374"<br>beim Hoch- oder Herunterladen.<br>Details: Read timed out</html>

Attachments (4)

2021-07-25 21_18_21_Verdächtige Daten gefunden. Trotzdem hochladen__Michael.png (20.1 KB) - added by DodalerAfenger 4 months ago.
ConditionalKeys.java (12.4 KB) - added by DodalerAfenger 4 months ago.
org.openstreetmap.josm.data.validation.tests
josm_21163.patch (6.0 KB) - added by DodalerAfenger 4 months ago.
Adopting checks in validation for key combinations of lanes and conditional generating false positive warnings before. E.g. tagging a way with "access:lanes:forward:conditional=yes|no @ (Mo-Fr 06:30-19:00); yes|no @ (Mo-Fr 06:30-19:00)" is now checked valid.
Lanes.java (5.1 KB) - added by DodalerAfenger 4 months ago.
org.openstreetmap.josm.data.validation.tests

Download all attachments as: .zip

Change History (13)

comment:1 Changed 4 months ago by skyper

Keywords: conditional highway lanes added
Version: tested
bus:lanes:conditional=yes|designated @ (Mo-Fr 06:30-19:00)
cycleway:right:conditional=share_busway @ (Mo-Fr 06:30-19:00)
lanes:bus:forward:conditional=1 @ (Mo-Fr 06:30-19:00)
parking:condition:right:conditional=free @ (Mo-Fr 19:00-22:00);free @ (Sa 08:00-22:00)

Right, all are valid and only parking:condition:right:conditional could profit from a white space in front of the second value or be summarized to one value: free @ (Mo-Fr 19:00-22:00, Sa 08:00-22:00).

By the way, the tagging of the way is inconsistent and a mix of two different systems. :lanes-tagging is incorrect as at least :forward/:backward are missing with some keys and access:lanes:forward:conditional plus bicycle:lanes:forward:conditional are missing. Adding lanes:backward=1 is appreciated in such cases, too.

comment:2 Changed 4 months ago by skyper

Similar to #9679.

comment:3 Changed 4 months ago by DodalerAfenger

Thanks for your feedback!

Hopefully it does not take another 7 years to make the validator a little less restrict as

Similar to #9679

may imply. ;-)

By the way, the tagging of the way is inconsistent and a mix of two different systems. :lanes-tagging is incorrect as at least :forward/:backward are missing with some keys and access:lanes:forward:conditional plus bicycle:lanes:forward:conditional are missing. Adding lanes:backward=1 is appreciated in such cases, too.

I will look into that. But I am not that much into mapping/tagging complex stuff, yet.

comment:4 Changed 4 months ago by DodalerAfenger

I have worked on the adaption of the java code to address this issue myself.

This is my first work on the project and as well with Java. I did some programming (Perl/Python/Javascript/... scripting) before, but never worked in a huge project like this one before. So please, feel free to have a look at the code when it shows up in the repository and let me know if I can improve anything. Your input is strongly welcome.

I already place this information here, since it seams to be good practice to reference tickets within the commit comments.

Before startet my work, I checked taginfo for information on how often the combination lanes/conditional is used. I place it below, in case anyone is interested in this, too. For me it was used often enough, to adopt the checks in JOSM ;-)

Edit:
Ok, it is not THAT easy after all to commit code to the repo. My user account does not have the appropriate rights and I then came across this note via here. So, as mentioned there I will simply attach my work to this ticket. It would be sad not to see the changes become productive. Please leave a short comment when you "found" the attached code. :)

btw, that would have been my commit comment:
see #21163 Adopting checks in validation for key combinations of lanes and conditional generating false positive warnings before. E.g. tagging a way with "access:lanes:forward:conditional=yes|no @ (Mo-Fr 06:30-19:00); yes|no @ (Mo-Fr 06:30-19:00)" is now checked valid.

key occurences
lanes:bus:conditional 3317
access:lanes:conditional 627
psv:lanes:conditional 611
lanes:psv:conditional 511
hgv:lanes:conditional 427
goods:lanes:conditional 272
lanes:conditional 213
lanes:backward:conditional 210
lanes:forward:conditional 181
motor_vehicle:lanes:conditional 147
lanes:psv:forward:conditional 144
motorcycle:lanes:conditional 97
bus:lanes:conditional 87
bicycle:lanes:conditional 82
lanes:psv:backward:conditional 79
cycleway:lanes:conditional 74
motorcar:lanes:conditional 55
turn:lanes:conditional 46
access:lanes:forward:conditional 40
hov:lanes:conditional 39
lanes:bus:forward:conditional 31
psv:lanes:backward:conditional 25
tourist_bus:lanes:conditional 21
coach:lanes:conditional 21
access:lanes:backward:conditional 20
bus:lanes:forward:conditional 18
motor_vehicle:lanes:forward:conditional 15
motor_vehicle:lanes:backward:conditional 13
destination:lanes:conditional 12
lanes:taxi:conditional 12
hgv:lanes:forward:conditional 12
lanes:motorcycle:conditional 12
lanes:bicycle:conditional 12
vehicle:lanes:conditional 11
lgv:lanes:conditional 10
bicycle:lanes:forward:conditional 10
cycleway:lanes:backward:conditional 10
lanes:tourist_bus:conditional 9
maxspeed:lanes:conditional 9
lanes:coach:conditional 9
turn:lanes:forward:conditional 9
motorcycle:lanes:backward:conditional 8
lanes:bus:backward:conditional 7
lanes:cycleway:conditional 7
turn:lanes:backward:conditional 7
lanes:goods:conditional 6
maxspeed:lanes:forward:conditional 6
bicycle:lanes:backward:conditional 6
lanes:hgv:conditional 6
lanes:hov:conditional 6
was:lanes:psv:conditional 5
vehicle:lanes:forward:conditional 4
turn:motor_vehicle:lanes:conditional 3
taxi:lanes:forward:conditional 3
moped:lanes:conditional 3
psv:lanes:forward:conditional 3
emergency:lanes:conditional 2
note:lanes:conditional 2
lanes:motorcycle:backward:conditional 2
hov:lanes:backward:conditional 2
maxspeed:hgv:lanes:conditional 1
vehicle:lanes:backward:conditional 1
lanes:emergency:conditional 1
lanes:motorcycle:forward:conditional 1
was:bus:lanes:conditional 1
Last edited 4 months ago by DodalerAfenger (previous) (diff)

Changed 4 months ago by DodalerAfenger

Attachment: ConditionalKeys.java added

org.openstreetmap.josm.data.validation.tests

Changed 4 months ago by DodalerAfenger

Attachment: josm_21163.patch added

Adopting checks in validation for key combinations of lanes and conditional generating false positive warnings before. E.g. tagging a way with "access:lanes:forward:conditional=yes|no @ (Mo-Fr 06:30-19:00); yes|no @ (Mo-Fr 06:30-19:00)" is now checked valid.

Changed 4 months ago by DodalerAfenger

Attachment: Lanes.java added

org.openstreetmap.josm.data.validation.tests

comment:5 in reply to:  4 Changed 4 months ago by skyper

Summary: Validating an object (way with conditionals) fails[Pacth ]Validating an object (way with conditionals) fails

Replying to DodalerAfenger:

Before startet my work, I checked taginfo for information on how often the combination lanes/conditional is used. I place it below, in case anyone is interested in this, too. For me it was used often enough, to adopt the checks in JOSM ;-)

Be careful, these tags are mixed from two different tagging systems. lanes=* plus lanes:[forward/backward]=* is used by both but lanes:[VEHICLE_TYPE]*=* has only numbers as values while *:lanes* is the :lanes-tagging-schema and has the mentioned value combinations with multiple values with inner separator ; and outer separator | like turn:lanes:forward:conditional=left;through|through;right @ (Mo-Fr 08:00-10:00, Sa 07:00-09:00; PH off). [VEHICLE_TYPE]:lanes* as all access tags should only have one inner value like hgv:lanes:conditional=no|yes|destination @ (Mo-Fr 00-00-06:00,22:00-24:00; PH,Sa,Su).

Edit:
Ok, it is not THAT easy after all to commit code to the repo. My user account does not have the appropriate rights and I then came across this note via here. So, as mentioned there I will simply attach my work to this ticket. It would be sad not to see the changes become productive. Please leave a short comment when you "found" the attached code. :)

Patches are welcome.
Please, always add [Patch] to the summary to have the ticket listed under Open tickets with patches attached.

btw, that would have been my commit comment:
see #21163 Adopting checks in validation for key combinations of lanes and conditional generating false positive warnings before. E.g. tagging a way with "access:lanes:forward:conditional=yes|no @ (Mo-Fr 06:30-19:00); yes|no @ (Mo-Fr 06:30-19:00)" is now checked valid.

Mmh, access:lanes:forward:conditional=yes|no @ (Mo-Fr 06:30-19:00); yes|no @ (Mo-Fr 06:30-19:00) is incorrect in my eyes as twice the same value is strange. But I guess this is a general problem of the conditional tags so far, as the time spans are only checked singular and not as combination.

comment:6 Changed 4 months ago by skyper

Summary: [Pacth ]Validating an object (way with conditionals) fails[Patch] Validating an object (way with conditionals) fails

comment:7 Changed 4 months ago by DodalerAfenger

Be careful, these tags are mixed from two different tagging systems.

Thanks for that hint. I was aware of that since i checked this site. But I did not set up my checks in a way that it fits both. Currently it would allow | separators for lanes:[VEHICLE_TYPE]*=*. I will look into that again.

with multiple values with inner separator ; and outer separator |

I was not aware of the inner separator ;. Now I found the spec here. (Just adding it here for reference...)

Patches are welcome. Please, always add [Patch] to the summary to have the ticket listed under Open tickets with patches attached.

Ok. I will do so, when I fixed the stuff mentioned just above. Until that I will remove [Patch] from ticket subject again.

Mmh, access:lanes:forward:conditional=yes|no @ (Mo-Fr 06:30-19:00); yes|no @ (Mo-Fr 06:30-19:00) is incorrect in my eyes as twice the same value is strange. But I guess this is a general problem of the conditional tags so far, as the time spans are only checked singular and not as combination.

I only added that for showing the allowable format, not even thinking about checking overlapping time spans. But, I will also spend some moments on how checking this. ;-)

Thanks for your feedback!

Last edited 4 months ago by DodalerAfenger (previous) (diff)

comment:8 Changed 4 months ago by DodalerAfenger

Summary: [Patch] Validating an object (way with conditionals) failsValidating an object (way with conditionals) fails

comment:9 in reply to:  7 Changed 4 months ago by skyper

Replying to DodalerAfenger:

Be careful, these tags are mixed from two different tagging systems.

Thanks for that hint. I was aware of that since i checked this site. But I did not set up my checks in a way that it fits both. Currently it would allow | separators for lanes:[VEHICLE_TYPE]*=*. I will look into that again.

Basically, lanes=*, lanes:[forward|backward|both_ways]=* and lanes:[VEHICLE_TYPE]*=* have only a single, positive number as value. I thought half lanes are sometimes mapped, e.g. 1.5 is OK, but the wiki, taginfo and JOSM validator tell a different story.
Anyway lanes:[VEHICLE_TYPE]*=* is not included in numeric.mapcss and would need some lines of new code.

with multiple values with inner separator ; and outer separator |

I was not aware of the inner separator ;. Now I found the spec here. (Just adding it here for reference...)

Right, the inner separator is only used with a few keys like e.g. turn and destination + subtags. Basically, all keys which may have multiple values in their basic form can have multiple values within the outer separator.

Patches are welcome. Please, always add [Patch] to the summary to have the ticket listed under Open tickets with patches attached.

Ok. I will do so, when I fixed the stuff mentioned just above. Until that I will remove [Patch] from ticket subject again.

Fine. Some use [WIP Patch] instead of removing.

Mmh, access:lanes:forward:conditional=yes|no @ (Mo-Fr 06:30-19:00); yes|no @ (Mo-Fr 06:30-19:00) is incorrect in my eyes as twice the same value is strange. But I guess this is a general problem of the conditional tags so far, as the time spans are only checked singular and not as combination.

I only added that for showing the allowable format, not even thinking about checking overlapping time spans. But, I will also spend some moments on how checking this. ;-)

This gets really tricky and is probably worth an own ticket. Identical values like the example above or hov:lanes=conditional=yes|yes|designated @ (Mo-Sa 06:00-09:00); yes|yes|designated @ (Mo-Fr 14:00-18:00) might raise a warning about prettifying it into one value but digging deeper and interpreting the complete opening_hours part is some work, I guess. So far, I have to check my time spans each on its own at https://openingh.openstreetmap.de/evaluation_tool.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set.
to The owner will be changed from team to the specified user.
The owner will change to DodalerAfenger
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket
The owner will be changed from team to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.