r.in.pdal: Improve PDAL error handling #4750
Conversation
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: OSGeo#4157
marisn
added
raster
github-actions
bot
added
Python
|
Seems an appropriate solution to send nosrs to PDAL by reading your linked threads. Was the "nosrs" option available for all our supported versions? If not, we might need to change our version requirements. |
|
@marisn I didn't know you were the person of knowledge on that topic. In the QGIS thread linked, it suggested having PDAL >=2.7.2. We had a problem in our docker images, where the PDAL version couldn't be upgraded, and we reverted it. #3960 One of the docker images is stuck on 2.7.1 (ubuntu wxGUI), and the debian one on 2.7.2. |
Good catch! It has been added in PDAL 2.3.0 – we'll have to come up with some version check as PDAL doesn't like unknown options. Our minimum PDAL version is 1.7.1, although I have no idea if it is even possible to compile GRASS with all of its dependencies to work together with such ancient version. |
We don't need to plan supporting such incompatible versions. Just like the Linux kernel assumes that, at one point, some older hardware isn't expected to be running the latest and greatest. It is normal for software to increase the requirements range at each release. |
|
I changed the configure.ac file to check for PDAL version as I was unable to find a solution how to check if PDAL reader.las has a "nosrs" as a valid option. I tested the configure part and module with PDAL 1.8.0, 2.4.2, 2.4.3, 2.5.0 and current master. Only problem remaining is a need to regenerate files created with autoheader and autoconf – when I run those commands on my system, I get a huge list of changes in their generated files in comparison to current ones in the git. Hints how to tackle it? |
readers.las "nosrs" option makes sense only since 2.4.3 (if I read correctly PDAL/PDAL#3818), but that is not available even in Ubuntu 22.04. The "nosrs" option does not cause runtime error only since 2.3.0, thus would not work with Debian 10 and 11 (there is no official PDAL package in 12). Thus al though the most recent distributions are fine, PDAL <2.4.3 is not that uncommon yet. |
Would it be possible to use the macros
What is the version of your autoconf? |
Reduces overhead of the configure stage
You are right. This is a better approach. I was so focused on having a C++ code that performs the check during configure stage (and failing miserably to come up with one), that I missed the most obvious solution. Thanks!
autoconf (GNU Autoconf) 2.72 |
We could also replace existing check for |
Great, looks much better!
We're still at 2.71, that's why all the unrelated changes. |
|
It's nice to avoid having to port the autoconf/configure changes to cmake |
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 propagates SRS check override from GRASS to PDAL library thus eliminating an error observed by non-conformant 1.4 LAS files.
Fixes: #4157
See also:
qgis/QGIS#51862 (comment)
PDAL/PDAL#3818