From 856a5141ea392b620e23dbe486e595564e2fb629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C4=81ris=20Narti=C5=A1s?= Date: Tue, 3 Dec 2024 01:23:33 +0200 Subject: [PATCH] r.in.pdal: Improve PDAL error handling (#4750) * 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: https://github.com/OSGeo/grass/issues/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 --- raster/r.in.pdal/info.cpp | 52 +++++++- raster/r.in.pdal/info.h | 14 +++ raster/r.in.pdal/main.cpp | 38 +++++- .../testsuite/test_r_in_pdal_print.py | 113 ++++++++++++++++++ 4 files changed, 206 insertions(+), 11 deletions(-) create mode 100644 raster/r.in.pdal/testsuite/test_r_in_pdal_print.py diff --git a/raster/r.in.pdal/info.cpp b/raster/r.in.pdal/info.cpp index cd8fd6483ff..d9f506a9ace 100644 --- a/raster/r.in.pdal/info.cpp +++ b/raster/r.in.pdal/info.cpp @@ -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). @@ -12,8 +12,14 @@ #include "info.h" #include +#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; @@ -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(); @@ -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; @@ -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(); diff --git a/raster/r.in.pdal/info.h b/raster/r.in.pdal/info.h index 7f8d0138ab1..387efd21ef0 100644 --- a/raster/r.in.pdal/info.h +++ b/raster/r.in.pdal/info.h @@ -27,15 +27,29 @@ #pragma clang diagnostic pop #endif +#include +#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 #include #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 diff --git a/raster/r.in.pdal/main.cpp b/raster/r.in.pdal/main.cpp index 36d09558eea..fe17e80cb08 100644 --- a/raster/r.in.pdal/main.cpp +++ b/raster/r.in.pdal/main.cpp @@ -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 @@ -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); } @@ -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; @@ -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); @@ -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) { diff --git a/raster/r.in.pdal/testsuite/test_r_in_pdal_print.py b/raster/r.in.pdal/testsuite/test_r_in_pdal_print.py new file mode 100644 index 00000000000..2c613ab88e2 --- /dev/null +++ b/raster/r.in.pdal/testsuite/test_r_in_pdal_print.py @@ -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()