Skip to content

Commit

Permalink
r.in.pdal: Improve PDAL error handling (#4750)
Browse files Browse the repository at this point in the history
* r.in.pdal: Improve PDAL error handling
Some non-conformant LAS files trigger an error in PDAL library that
results in an unhandled exit from r.in.pdal module.
This code catches all PDAL errors and reports them in a GRASS way.

One of error sources is lack of SRS information in the imported LAS file.
This code propogates SRS check override from GRASS to PDAL library
thus eliminating an error observed by non-conformant 1.4 LAS files.

Fixes: #4157

* r.in.pdal: update copyright notice

* r.in.pdal: enable reader.las "nosrs" option only in PDAL >= 2.4.3

* r.in.pdal: use PDAL defined macros to determine its version
Reduces overhead of the configure stage

* r.in.pdal: remove debugging pritnout left by accident

* r.in.pdal: do not pass unused parameter to functions if "nosrs" is
not available
  • Loading branch information
marisn authored Dec 2, 2024
1 parent 80a574d commit 856a514
Show file tree
Hide file tree
Showing 4 changed files with 206 additions and 11 deletions.
52 changes: 47 additions & 5 deletions raster/r.in.pdal/info.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* r.in.pdal Functions printing out various information on input LAS files
*
* Copyright 2021 by Maris Nartiss, and The GRASS Development Team
* Copyright 2021-2024 by Maris Nartiss, and The GRASS Development Team
* Author: Maris Nartiss
*
* This program is free software licensed under the GPL (>=v2).
Expand All @@ -12,8 +12,14 @@
#include "info.h"
#include <cmath>

#ifdef PDAL_USE_NOSRS
void get_extent(struct StringList *infiles, double *min_x, double *max_x,
double *min_y, double *max_y, double *min_z, double *max_z,
bool nosrs)
#else
void get_extent(struct StringList *infiles, double *min_x, double *max_x,
double *min_y, double *max_y, double *min_z, double *max_z)
#endif
{
pdal::StageFactory factory;
bool first = 1;
Expand All @@ -25,15 +31,27 @@ void get_extent(struct StringList *infiles, double *min_x, double *max_x,

std::string pdal_read_driver = factory.inferReaderDriver(infile);
if (pdal_read_driver.empty())
G_fatal_error("Cannot determine input file type of <%s>", infile);
G_fatal_error(_("Cannot determine input file type of <%s>"),
infile);

pdal::PointTable table;
pdal::Options las_opts;
pdal::Option las_opt("filename", infile);
las_opts.add(las_opt);
#ifdef PDAL_USE_NOSRS
if (nosrs) {
pdal::Option nosrs_opt("nosrs", true);
las_opts.add(nosrs_opt);
}
#endif
pdal::LasReader las_reader;
las_reader.setOptions(las_opts);
las_reader.prepare(table);
try {
las_reader.prepare(table);
}
catch (const std::exception &err) {
G_fatal_error(_("PDAL error: %s"), err.what());
}
const pdal::LasHeader &las_header = las_reader.header();
if (first) {
*min_x = las_header.minX();
Expand Down Expand Up @@ -62,16 +80,28 @@ void get_extent(struct StringList *infiles, double *min_x, double *max_x,
}
}

#ifdef PDAL_USE_NOSRS
void print_extent(struct StringList *infiles, bool nosrs)
#else
void print_extent(struct StringList *infiles)
#endif
{
double min_x, max_x, min_y, max_y, min_z, max_z;

#ifdef PDAL_USE_NOSRS
get_extent(infiles, &min_x, &max_x, &min_y, &max_y, &min_z, &max_z, nosrs);
#else
get_extent(infiles, &min_x, &max_x, &min_y, &max_y, &min_z, &max_z);
#endif
fprintf(stdout, "n=%f s=%f e=%f w=%f b=%f t=%f\n", max_y, min_y, max_x,
min_x, min_z, max_z);
}

#ifdef PDAL_USE_NOSRS
void print_lasinfo(struct StringList *infiles, bool nosrs)
#else
void print_lasinfo(struct StringList *infiles)
#endif
{
pdal::StageFactory factory;
pdal::MetadataNode meta_node;
Expand All @@ -86,15 +116,27 @@ void print_lasinfo(struct StringList *infiles)

std::string pdal_read_driver = factory.inferReaderDriver(infile);
if (pdal_read_driver.empty())
G_fatal_error("Cannot determine input file type of <%s>", infile);
G_fatal_error(_("Cannot determine input file type of <%s>"),
infile);

pdal::PointTable table;
pdal::Options las_opts;
pdal::Option las_opt("filename", infile);
las_opts.add(las_opt);
#ifdef PDAL_USE_NOSRS
if (nosrs) {
pdal::Option nosrs_opt("nosrs", true);
las_opts.add(nosrs_opt);
}
#endif
pdal::LasReader las_reader;
las_reader.setOptions(las_opts);
las_reader.prepare(table);
try {
las_reader.prepare(table);
}
catch (const std::exception &err) {
G_fatal_error(_("PDAL error: %s"), err.what());
}
const pdal::LasHeader &h = las_reader.header();
pdal::PointLayoutPtr point_layout = table.layout();
const pdal::Dimension::IdList &dims = point_layout->dims();
Expand Down
14 changes: 14 additions & 0 deletions raster/r.in.pdal/info.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,29 @@
#pragma clang diagnostic pop
#endif

#include <pdal/pdal_features.hpp>
#if (PDAL_VERSION_MAJOR >= 2 && PDAL_VERSION_MINOR > 4) || \
(PDAL_VERSION_MAJOR == 2 && PDAL_VERSION_MINOR == 4 && \
PDAL_VERSION_PATCH == 3)
#define PDAL_USE_NOSRS 1
#endif

extern "C" {
#include <grass/gis.h>
#include <grass/glocale.h>
#include "string_list.h"
}

#ifdef PDAL_USE_NOSRS
void get_extent(struct StringList *, double *, double *, double *, double *,
double *, double *, bool);
void print_extent(struct StringList *, bool);
void print_lasinfo(struct StringList *, bool);
#else
void get_extent(struct StringList *, double *, double *, double *, double *,
double *, double *);
void print_extent(struct StringList *);
void print_lasinfo(struct StringList *);
#endif

#endif // INFO_H
38 changes: 32 additions & 6 deletions raster/r.in.pdal/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
*
* AUTHOR(S): Vaclav Petras
* Based on r.in.xyz and r.in.lidar by Markus Metz,
* Hamish Bowman, Volker Wichmann
* Hamish Bowman, Volker Wichmann, Maris Nartiss
*
* PURPOSE: Imports LAS LiDAR point clouds to a raster map using
* aggregate statistics.
*
* COPYRIGHT: (C) 2019-2021 by Vaclav Petras and the GRASS Development Team
* COPYRIGHT: (C) 2019-2024 by Vaclav Petras and the GRASS Development Team
*
* This program is free software under the GNU General Public
* License (>=v2). Read the file COPYING that comes with
Expand Down Expand Up @@ -446,12 +446,20 @@ int main(int argc, char *argv[])

/* If we print extent, there is no need to validate rest of the input */
if (print_extent_flag->answer) {
#ifdef PDAL_USE_NOSRS
print_extent(&infiles, over_flag->answer);
#else
print_extent(&infiles);
#endif
exit(EXIT_SUCCESS);
}

if (print_info_flag->answer) {
#ifdef PDAL_USE_NOSRS
print_lasinfo(&infiles, over_flag->answer);
#else
print_lasinfo(&infiles);
#endif
exit(EXIT_SUCCESS);
}

Expand Down Expand Up @@ -507,7 +515,12 @@ int main(int argc, char *argv[])
if (extents_flag->answer) {
double min_x, max_x, min_y, max_y, min_z, max_z;

#ifdef PDAL_USE_NOSRS
get_extent(&infiles, &min_x, &max_x, &min_y, &max_y, &min_z, &max_z,
over_flag->answer);
#else
get_extent(&infiles, &min_x, &max_x, &min_y, &max_y, &min_z, &max_z);
#endif

region.east = xmax = max_x;
region.west = xmin = min_x;
Expand Down Expand Up @@ -711,16 +724,24 @@ int main(int argc, char *argv[])

std::string pdal_read_driver = factory.inferReaderDriver(infile);
if (pdal_read_driver.empty())
G_fatal_error("Cannot determine input file type of <%s>", infile);
G_fatal_error(_("Cannot determine input file type of <%s>"),
infile);

pdal::Options las_opts;
pdal::Option las_opt("filename", infile);
las_opts.add(las_opt);
#ifdef PDAL_USE_NOSRS
if (over_flag->answer) {
pdal::Option nosrs_opt("nosrs", true);
las_opts.add(nosrs_opt);
}
#endif
// stages created by factory are destroyed with the factory
pdal::Stage *reader = factory.createStage(pdal_read_driver);
if (!reader)
G_fatal_error("PDAL reader creation failed, a wrong format of <%s>",
infile);
G_fatal_error(
_("PDAL reader creation failed, a wrong format of <%s>"),
infile);
reader->setOptions(las_opts);
readers.push_back(reader);
merge_filter.setInput(*reader);
Expand Down Expand Up @@ -779,7 +800,12 @@ int main(int argc, char *argv[])
// consumption, so using 10k in case it is faster for some cases
pdal::point_count_t point_table_capacity = 10000;
pdal::FixedPointTable point_table(point_table_capacity);
binning_writer.prepare(point_table);
try {
binning_writer.prepare(point_table);
}
catch (const std::exception &err) {
G_fatal_error(_("PDAL error: %s"), err.what());
}

// getting projection is possible only after prepare
if (over_flag->answer) {
Expand Down
113 changes: 113 additions & 0 deletions raster/r.in.pdal/testsuite/test_r_in_pdal_print.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
"""
Name: r.in.pdal info printing and error handling tests
Purpose: Validates output of LAS file property printing and handling
of broken LAS files
Author: Maris Nartiss
Copyright: (C) 2024 by Maris Nartiss and the GRASS Development Team
Licence: This program is free software under the GNU General Public
License (>=v2). Read the file COPYING that comes with GRASS
for details.
"""

import os
import pathlib
import shutil
import unittest
from tempfile import TemporaryDirectory

from grass.script import core as grass
from grass.script import read_command
from grass.gunittest.case import TestCase
from grass.gunittest.main import test


class InfoTest(TestCase):
"""
Test printing of extent and metadata
This test requires pdal CLI util to be available.
"""

@classmethod
@unittest.skipIf(shutil.which("pdal") is None, "Cannot find pdal utility")
def setUpClass(cls):
"""Ensures expected computational region and generated data"""
cls.use_temp_region()
cls.runModule("g.region", n=18, s=0, e=18, w=0, res=6)

cls.data_dir = os.path.join(pathlib.Path(__file__).parent.absolute(), "data")
cls.point_file = os.path.join(cls.data_dir, "points.csv")
cls.tmp_dir = TemporaryDirectory()
cls.las_file = os.path.join(cls.tmp_dir.name, "points.las")
grass.call(
[
"pdal",
"translate",
"-i",
cls.point_file,
"-o",
cls.las_file,
"-r",
"text",
"-w",
"las",
"--writers.las.format=0",
"--writers.las.extra_dims=all",
"--writers.las.minor_version=4",
]
)
cls.broken_las = os.path.join(cls.tmp_dir.name, "broken.las")
pathlib.Path(cls.broken_las).write_bytes(b"LASF")

@classmethod
def tearDownClass(cls):
"""Remove the temporary region and generated data"""
cls.tmp_dir.cleanup()
cls.del_temp_region()

@unittest.skipIf(shutil.which("r.in.pdal") is None, "Cannot find r.in.pdal")
def test_extent_bad(self):
"""A broken LAS file should result in an error"""
self.assertModuleFail("r.in.pdal", input=self.broken_las, flags="g", quiet=True)

@unittest.skipIf(shutil.which("r.in.pdal") is None, "Cannot find r.in.pdal")
def test_info_bad(self):
"""A broken LAS file should result in an error"""
self.assertModuleFail("r.in.pdal", input=self.broken_las, flags="p", quiet=True)

@unittest.skipIf(shutil.which("r.in.pdal") is None, "Cannot find r.in.pdal")
def test_extent_good(self):
"""Reported extent should match provided data"""
out = read_command("r.in.pdal", input=self.las_file, flags="g", quiet=True)
for kvp in out.strip().split(" "):
key, value = kvp.split("=")
if key == "n":
self.assertAlmostEqual(float(value), 17, places=6)
continue
if key == "s":
self.assertAlmostEqual(float(value), 1, places=6)
continue
if key == "e":
self.assertAlmostEqual(float(value), 17, places=6)
continue
if key == "w":
self.assertAlmostEqual(float(value), 1, places=6)
continue
if key == "t":
self.assertAlmostEqual(float(value), 28, places=6)
continue
if key == "b":
self.assertAlmostEqual(float(value), 1, places=6)

@unittest.skipIf(shutil.which("r.in.pdal") is None, "Cannot find r.in.pdal")
def test_info_good(self):
"""Validate successful file info printing"""
out = read_command("r.in.pdal", input=self.las_file, flags="p", quiet=True)
self.assertIn("File version = 1.4", out)
self.assertIn("File signature: LASF", out)
self.assertIn("Point count: 53", out)


if __name__ == "__main__":
test()

0 comments on commit 856a514

Please sign in to comment.