Opened 4 years ago
Last modified 3 years ago
#21163 new defect
[WIP Patch] 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: | angoca |
Description
What steps will reproduce the problem?
- 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)
Change History (16)
by , 4 years ago
comment:1 by , 4 years ago
| Keywords: | conditional highway lanes added |
|---|---|
| Version: | → tested |
comment:3 by , 4 years ago
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.
follow-up: 5 comment:4 by , 4 years ago
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 |
by , 4 years ago
| Attachment: | ConditionalKeys.java added |
|---|
org.openstreetmap.josm.data.validation.tests
by , 4 years ago
| 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.
comment:5 by , 4 years ago
| 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 by , 4 years ago
| Summary: | [Pacth ]Validating an object (way with conditionals) fails → [Patch] Validating an object (way with conditionals) fails |
|---|
follow-up: 9 comment:7 by , 4 years ago
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!
comment:8 by , 4 years ago
| Summary: | [Patch] Validating an object (way with conditionals) fails → Validating an object (way with conditionals) fails |
|---|
comment:9 by , 4 years ago
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 forlanes:[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.
comment:10 by , 3 years ago
| Summary: | Validating an object (way with conditionals) fails → [WIP Patch] Validating an object (way with conditionals) fails |
|---|
Right, all are valid and only
parking:condition:right:conditionalcould 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/:backwardare missing with some keys andaccess:lanes:forward:conditionalplusbicycle:lanes:forward:conditionalare missing. Addinglanes:backward=1is appreciated in such cases, too.