Modify

Opened 8 days ago

Last modified 4 days ago

#24851 new defect

[Patch] Power line checks won't report existing non-power nodes

Reported by: francois.lacombe Owned by: GerdP
Priority: normal Milestone: 26.09
Component: Core validator Version:
Keywords: power Cc: gaben

Description

Hello

I found out that current power line checks won't report a power line passing by an exiting node of a highway, for instance.

Here the sketch:
I draw a power line and instead of inadvertently connecting it to a highway with a new node, I inadvertently click on an existing node of this highway.
The highway's node will remain unchanged, no new tag, so it won't be reported in "node without power" check nor "Node connects a power line or cable with an object".

Those checks are currently done in the Java class PowerLines.java. I didn't studied it in detail yet but I bet it would be possible to move part of it to mapcss like

way[power=line] > node[!power],
  throwWarning: tr("{0} node without power");
}

If correct, this would allow to strip a bit of specific code and make validation more efficient.

Regarding connections between power lines and highways or waterways, I'm not sure we can achieve them in pure mapcss. Let me know.

Best regards

Attachments (2)

24851-powerlines.patch (4.7 KB ) - added by GerdP 4 days ago.
Patch for test PowerLines so that it adds the related parent objects to the error node
24851-powerlines-v2.patch (5.3 KB ) - added by GerdP 4 days ago.
improved patch

Download all attachments as: .zip

Change History (10)

comment:1 by GerdP, 8 days ago

Owner: changed from team to GerdP
Type: enhancementdefect

I see. You don't get a warning on Upload, only when you do a full check. So I think this is a bug in class PowerLines, possible a regression of r17111.

Version 0, edited 8 days ago by GerdP (next)

comment:2 by francois.lacombe, 8 days ago

Thank you, I indeed get the warning with a full check.

I didn't noticed we already moved from mapcss to java code, so I'm not sure the opposite move is welcome.
By the way r17111 shows what was done before about intersections with building with pure mapcss.

comment:3 by GerdP, 8 days ago

Seems the problem is a regression of #23397. If you set preference validator.partial.removeIrrelevant to false you also get the messages on upload. I'll dig deeper into this...

comment:4 by GerdP, 6 days ago

The problem is that the check in class PowerLines creates error messages which do not contain the power line way. Same problem occurs when you draw a new highway=* using e.g. an existing power=pole node.
I see different solutions:
1) Change class PowerLines so that it adds the corresponding way
2) Add a filter for those error types which should never be treated as irrelevant, e.g. the errors from class PowerLines using the code 2501 or 2502.

In both cases the question is how many more tests we have where this problem occurs and how to find them...

comment:5 by francois.lacombe, 5 days ago

Can I have 1) and 2) please? :)

More seriously, 1) first and let 2) for a more global cleanup if needed

by GerdP, 4 days ago

Attachment: 24851-powerlines.patch added

Patch for test PowerLines so that it adds the related parent objects to the error node

comment:6 by GerdP, 4 days ago

Cc: gaben added
Summary: Power line checks won't report existing non-power nodes[Patch] Power line checks won't report existing non-power nodes

comment:7 by gaben, 4 days ago

Milestone: 26.09

Hmm, I have to test it but looks good.

Last edited 4 days ago by gaben (previous) (diff)

comment:8 by GerdP, 4 days ago

Thinking again about it maybe the unrelatedParents should only be collected when nodeInLineOrCable is true, else this creates a lot of work for the garbage collector.

by GerdP, 4 days ago

Attachment: 24851-powerlines-v2.patch added

improved patch

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain GerdP.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from GerdP to the specified user.
Next status will be 'needinfo'. The owner will be changed from GerdP to francois.lacombe.
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 GerdP 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.