Skip to content

Commit

Permalink
Exclude nodes with kerb=no from missing curb_nodes validation
Browse files Browse the repository at this point in the history 
(closes #1657)
  • Loading branch information
@bhousel
bhousel committed Jan 7, 2025
1 parent f286532 commit d784b2d
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 61 deletions.
22 changes: 11 additions & 11 deletions modules/validations/curb_nodes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ export function validationCurbNodes(context) {
const wayID = way.id;
if (!hasRoutableTags(way) || !isCrossingWay(way.tags)) return issues;

const firstNode = graph.entity(way.nodes[0]);
const lastNode = graph.entity(way.nodes[way.nodes.length - 1]);
const firstNodeHasCurb = hasCurbNode(firstNode, graph);
const lastNodeHasCurb = hasCurbNode(lastNode, graph);
const firstNode = graph.entity(way.nodes.at(0));
const lastNode = graph.entity(way.nodes.at(-1));
const firstNodeHasCurb = hasCurbTag(firstNode);
const lastNodeHasCurb = hasCurbTag(lastNode);

// Check if either end is missing a curb
if (!firstNodeHasCurb || !lastNodeHasCurb) {
Expand Down Expand Up @@ -129,14 +129,14 @@ export function validationCurbNodes(context) {


/**
* hasCurbNode
* hasCurbTag
* Checks if the given node has a curb
* @param {Node} node - The node entity to check
* @param {Graph} graph - The graph containing the node data
* @return {Boolean} True if the node has a curb, false otherwise
* @param {Node} node - The node entity to check
* @return {Boolean} true if the node has some tags that would indicate a curb, false otherwise
*/
function hasCurbNode(node, graph) {
return node.tags && node.tags.barrier === 'kerb';
function hasCurbTag(node) {
const tags = node.tags;
return !!tags.kerb || tags.barrier === 'kerb';
}


Expand Down Expand Up @@ -187,7 +187,7 @@ export function validationCurbNodes(context) {
* @param {Object} tags - The tags to assign to the new curb node.
*/
function insertCurbNode(node, way, graph, curbTags) {
if (hasCurbNode(node, graph)) return; // Exit if curb already exists
if (hasCurbTag(node)) return; // Exit if curb already exists

// Calculate the position for the new curb node
const nodeIndex = way.nodes.indexOf(node.id);
Expand Down
95 changes: 45 additions & 50 deletions test/browser/validations/curb_nodes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,21 @@ describe('validationCurbNodes', () => {
return issues;
}

it('has no errors on init', () => {
const issues = validate();
expect(issues).to.have.lengthOf(0);
});

function createWaysWithOneCrossingPoint(w1tags = {}, w2tags = {}) {
const n1 = Rapid.osmNode({ id: 'n-1', loc: [0, -1] });
const n2 = Rapid.osmNode({ id: 'n-2', loc: [0, 1] });
const n5 = Rapid.osmNode({ id: 'n-5', loc: [0, 0] });
const w1 = Rapid.osmWay({ id: 'w-1', nodes: ['n-1', 'n-5', 'n-2'], tags: w1tags });

const n3 = Rapid.osmNode({ id: 'n-3', loc: [-1, 0] });
const n4 = Rapid.osmNode({ id: 'n-4', loc: [ 1, 0] });
const w2 = Rapid.osmWay({ id: 'w-2', nodes: ['n-3', 'n-5', 'n-4'], tags: w2tags });

function createSingleCrossing(w1tags = {}, w2tags = {}, n1tags = {}, n2tags = {}) {
//
// n2 (w1 = the footway)
// |
// n3 -- n5 -- n4 (w2 = the road)
// |
// n1
//
const n1 = Rapid.osmNode({ id: 'n1', loc: [0, -1], tags: n1tags });
const n2 = Rapid.osmNode({ id: 'n2', loc: [0, 1], tags: n2tags });
const n3 = Rapid.osmNode({ id: 'n3', loc: [-1, 0] });
const n4 = Rapid.osmNode({ id: 'n4', loc: [ 1, 0] });
const n5 = Rapid.osmNode({ id: 'n5', loc: [0, 0] }); // road-crossing junction
const w1 = Rapid.osmWay({ id: 'w1', nodes: ['n1', 'n5', 'n2'], tags: w1tags });
const w2 = Rapid.osmWay({ id: 'w2', nodes: ['n3', 'n5', 'n4'], tags: w2tags });
const entities = [n1, n2, n3, n4, n5, w1, w2];
graph = new Rapid.Graph(entities);
tree = new Rapid.Tree(graph);
Expand All @@ -69,56 +69,51 @@ describe('validationCurbNodes', () => {
expect(issue.type).to.eql('curb_nodes');
expect(issue.severity).to.eql('suggestion');
expect(issue.entityIds).to.have.lengthOf(1);
expect(issue.entityIds[0]).to.eql('w-1');
expect(issue.entityIds[0]).to.eql('w1');
}

it('ignores untagged line crossing untagged line', () => {
createWaysWithOneCrossingPoint({}, {});
it('has no errors on init', () => {
const issues = validate();
expect(issues).to.have.lengthOf(0);
});

it('flags a crossing way and residential street if the street has no curb nodes', () => {
createWaysWithOneCrossingPoint({ highway: 'footway', footway: 'crossing' }, { highway: 'residential' });
const issues = validate();
verifySingleCurbNodeIssue(issues);
});

it('flags a crossing way with no curb nodes on a primary road', () => {
createWaysWithOneCrossingPoint({ highway: 'footway', footway: 'crossing' }, { highway: 'primary' });
it('ignores untagged line crossing untagged line', () => {
createSingleCrossing(); // no tags
const issues = validate();
verifySingleCurbNodeIssue(issues);
expect(issues).to.have.lengthOf(0);
});

it('ignores a crossing way with existing curb nodes at both ends', () => {
// Define nodes with curb tags at both ends
const n1 = Rapid.osmNode({ id: 'n-1', loc: [0, -1], tags: { barrier: 'kerb' } });
const n2 = Rapid.osmNode({ id: 'n-2', loc: [0, 1], tags: { barrier: 'kerb' } });
const n5 = Rapid.osmNode({ id: 'n-5', loc: [0, 0] }); // Middle node without curb
const w1 = Rapid.osmWay({ id: 'w-1', nodes: ['n-1', 'n-5', 'n-2'], tags: { highway: 'footway', footway: 'crossing' } });

const n3 = Rapid.osmNode({ id: 'n-3', loc: [-1, 0] });
const n4 = Rapid.osmNode({ id: 'n-4', loc: [ 1, 0] });
const w2 = Rapid.osmWay({ id: 'w-2', nodes: ['n-3', 'n-5', 'n-4'], tags: { highway: 'residential' } });

const entities = [n1, n2, n3, n4, n5, w1, w2];
graph = new Rapid.Graph(entities);
tree = new Rapid.Tree(graph);
tree.rebase(entities, true);
for (const type of ['primary', 'secondary', 'tertiary', 'residential']) {
it(`flags missing curb nodes on a crossing way and ${type} road`, () => {
createSingleCrossing(
{ highway: 'footway', footway: 'crossing' },
{ highway: type }
);
const issues = validate();
verifySingleCurbNodeIssue(issues);
});
}

it('ignores a crossing way with `barrier=kerb` tags at both ends', () => {
createSingleCrossing(
{ highway: 'footway', footway: 'crossing' },
{ highway: 'residential' },
{ barrier: 'kerb' },
{ barrier: 'kerb' }
);
const issues = validate();
expect(issues).to.have.lengthOf(0);
});

it('flags a crossing way with a missing curb node on a secondary road', () => {
createWaysWithOneCrossingPoint({ highway: 'footway', footway: 'crossing' }, { highway: 'secondary' });
it('ignores a crossing way with `kerb=*` tags at both ends', () => {
createSingleCrossing(
{ highway: 'footway', footway: 'crossing' },
{ highway: 'residential' },
{ kerb: 'no' },
{ kerb: 'maybe' }
);
const issues = validate();
verifySingleCurbNodeIssue(issues);
expect(issues).to.have.lengthOf(0);
});

it('flags a crossing way with no curb nodes on a tertiary road', () => {
createWaysWithOneCrossingPoint({ highway: 'footway', footway: 'crossing' }, { highway: 'tertiary' });
const issues = validate();
verifySingleCurbNodeIssue(issues);
});
});

0 comments on commit d784b2d

Please sign in to comment.