Dear folks,

after having hands-on at Splat! and considering myself as being still somehow unbiased, I would suggest to change a few things in future 2.0 release that will break backward compatibility:

• Give all parameters in SI units as default. That includes: lengths/heights in meter, frequency in Hz (but e.g. with correct parsing of 433M or 1.8G) freq in MHz should be fine
• Use standard geographical coordinates. The zero meridian goes through Greenwich. Eastern is positive, Western negative longitude. Alternatively, parse 122.4192W correctly, if Americans have a strong feeling about "negative" coordinates.
• Abolish annotated map for coverage predictions. The technical possibilities changed during the last 20 years and a georeferenced image can be easily loaded into web maps or offline GIS nowadays. (Wait for my work done in #5...)
• Abolish in-picture legend.
• Abolish "-hd" option. Nowadays, a global 30m DEM is standard and the old resolution of 90m is superseded.
• Abolish "-sdelim" and just use "_" as default delimiter. Or even better: read geotiff DEMs.
• Rename "-c" to "-los" as this calculates a line of sight coverage.
• Rename "-L" to "-radio" as this calculates a radio propagation coverage.
• Use EIRP instead of ERP - or just TX power and antenna pattern? (see #24).
• Use logarithmic antenna pattern instead of normalized voltage (!) radiation patterns (see #24).

Please consider my thoughts as proposals only. It's just a good time to reconsider the status quo. I won't break up with Splat! if you have different plans. 😃

Please also note that I will edit and add items from the list above in the future.

The Windows version does not handle spaces in path names gracefully. For example:

K:\hoche\Documents\C++ Projects\Splat\splat.EXE -hd -d ....\srtm1 -t tx -L 10.0 -R 15 -o K:\hoche\Documents\C++ Projects\Splat\test\tests\coverage_1deg_15mi_itm\coverage

            --==[ Welcome To SPLAT! MT HD v1.5.0b3 ]==--

Loading "....\srtm1/40_41_111_112-hd.sdf.bz2"... Done!
Loading tx.lrp
Default parameters have been assumed for this analysis.

Computing ITM path loss contours of "tx"
out to a radius of 15.00 miles with an RX antenna at 10.00 feet AGL

...

0% to 25% .oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo
25% to 50% .oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo
50% to 75% .oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo
75% to 100% .oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo.oOo
Done!

Site analysis report written to: "tx-site_report.txt"

Writing LR map "K:\hoche\Documents\C++.png" (3600x3630 image)... Done!

The png file got written to the wrong path.

Use clang-format tool (part of the LLVM projects) to enforce a consistent style for new and refactored code.

Add a pre-commit hook to ensure style is consistent. The issue was opened to allow for discussion, PR for consideration forthcoming.

As far as I can overview it to this point, @watkipet rewrote most of the code in a nice native C++ style with classes and objects. However, the differences with @hoche's code must be merged totally manually.

Furthermore, a standard folder structure (as of 2020) should be used (see https://github.com/kriasoft/Folder-Structure-Conventions/blob/master/README.md):

splat/
├── build/
├── demo/
├── docs/
├── include/
├── src/
├── utils/
├── AUTHORS
├── CHANGELOG
├── CMakeLists.txt
├── COPYING
└── README.md

That includes: lengths/heights in meter, frequency in Hz (but e.g. with correct parsing of 433M or 1.8G) freq in MHz should be fine.

2.0-alpha faults on FreeBSD (built using clang)

Using 8 threads...

 0% to  25% Process 55587 stopped
* thread #7, name = 'splat', stop reason = signal SIGSEGV: invalid address (fault address: 0xa0)
    frame #0: 0x0000000801c3d96f libc.so.7`_flockfile + 15
libc.so.7`_flockfile:
->  0x801c3d96f <+15>: cmpq   %rax, 0xa0(%rbx)
    0x801c3d976 <+22>: je     0x801c3d99a               ; <+58>
    0x801c3d978 <+24>: movq   %rax, %r14
    0x801c3d97b <+27>: leaq   0x98(%rbx), %rdi
(lldb) bt
* thread #7, name = 'splat', stop reason = signal SIGSEGV: invalid address (fault address: 0xa0)
  * frame #0: 0x0000000801c3d96f libc.so.7`_flockfile + 15
    frame #1: 0x0000000801c41fff libc.so.7`fwrite + 63
    frame #2: 0x000000000021d58e splat`ElevationMap::PlotLRPath(Site const&, Site const&, unsigned char, __sFILE*, AntennaPattern const&, Lrp const&) + 3486
    frame #3: 0x000000000021e7f0 splat`std::__1::__function::__func<std::__1::__bind<void (ElevationMap::*)(Site const&, Site const&, unsigned char, __sFILE*, AntennaPattern const&, Lrp const&), ElevationMap*, Site const&, Site&, unsigned char&, __sFILE*&, std::__1::reference_wrapper<AntennaPattern const>, Lrp const&>, std::__1::allocator<std::__1::__bind<void (ElevationMap::*)(Site const&, Site const&, unsigned char, __sFILE*, AntennaPattern const&, Lrp const&), ElevationMap*, Site const&, Site&, unsigned char&, __sFILE*&, std::__1::reference_wrapper<AntennaPattern const>, Lrp const&> >, void ()>::operator()() + 80
    frame #4: 0x000000000023f20c splat`WorkQueue::doWork() + 444
    frame #5: 0x000000000023fd3d splat`void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (WorkQueue::*) (), WorkQueue*> >(void*) + 61
    frame #6: 0x000000080039c82b libthr.so.3`___lldb_unnamed_symbol11$$libthr.so.3 + 331
(lldb)

edit notice: The primary goal of having a JSON output file for georeferencing PNG images is now superseded by the idea of a multi-purpose input/output functionality. The discussion below develops accordingly.

An interactive map that shows the coverage simulations is a useful feature. Splat will generate a HTML file that can be opened locally and just adds a Leaflet map with various OpenStreetMap tiles as background.
Further requirements:

• The output must be PNG, JPEG or GeoTIFF
• An additional json file is written that includes the simulation parameters (qth, lrp) and the extent of the image for georeferencing.

I implemented that already, but need to change some things and will push it next week.

The default output of Splat maps is in EPSG:4326 projection. Nowadays, most online maps (typical OpenStreetMap maps or Google Maps) are in EPSG:3857. Up to now Splat users had to transform the output on their own, for example with gdalwarp, after georeferencing the output png as a geotif:
gdal_translate -of Gtiff -a_ullr $x1 $y1 $x2 $y2 -a_srs EPSG:4326 map.png map_4326.tif (not necessary any more when using -tif output)
gdalwarp -s_srs EPSG:4326 -t_srs EPSG:3857 map_4326.tif map_3857.tif
Should Splat produce EPSG:3857 projected maps as default?

The zero meridian goes through Greenwich. Eastern is positive, Western negative longitude. Alternatively, parse 122.4192W correctly, if Americans have a strong feeling about "negative" coordinates.

Notes:

• GitHub's free level service is 500MB and 2000min/month of CPU time.
• They have VMs for Windows Server 2019, Ubuntu 18.04, Ubuntu 16.04, and MacOS 10.15.
• Windows CPU time is billed at 2x Linux time, and MacOS time is billed at 10x.

This is probably good enough to do a basic per-checkin level qualification, but the limited number of OSes available and very limited disk space means that it's not sufficient to do a full qualification, particularly since running splat-testsuite is both a CPU hog and requires about 400MB of disk.

I have a number of VMs (Vagrants) already set up on a moderately high-end host (Core i7-9700, 32GB, SSD), so I'm thinking I'll set that up to do automatically do extended checking and testing runs on that. I'll have to figure out how to report back errors though.

It would be nice if there were some basic usage instructions that could be activated with a --help switch like most other command line programs.

It would be useful to have an output mode where the dBm or dBuV/m can have more than 32 steps. Given we output at least 24bit image files, we easily increase the number of contours.

Further, if we do this within the GeoTIFF realm, we can multiband, and have the 1 file contain both path loss as well as field strength information. This GeoTIFF can easily then be used by other tools like Rio-RGB to produce datasets that can be used for high performance, mapping through for example Mapbox Terrain-RGB inspired frameworks accessible through Mapbox-GL for both the web and native

2.0-alpha doesn't build - it looks like there's a commandline parsing routine that was expected to set the "args" variable, but it got commented out. If that parser isn't stable we should leave it and everything that depends on it out.

~/src/splat/build$ make
[  2%] Building CXX object src/CMakeFiles/splat.dir/main.cpp.o
/home/hoche/src/splat/src/main.cpp: In function ‘int main(int, const char**)’:
/home/hoche/src/splat/src/main.cpp:1204:24: error: ‘args’ was not declared in this scope; did you mean ‘argc’?
 1204 |         json.WriteJSON(args, tx_site[0], lrp, mapfile);
      |                        ^~~~
      |                        argc
make[2]: *** [src/CMakeFiles/splat.dir/build.make:225: src/CMakeFiles/splat.dir/main.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:216: src/CMakeFiles/splat.dir/all] Error 2
make: *** [Makefile:149: all] Error 2

2.0-alpha has some example support for JSON.

• Strongly consider using a library instead of rolling our own. Every line of code we write comes with maintenance cost. Consider using a library that doesn't assume a particular development framework (such as Boost, Qt, or GLib). Consider using a library that is available in most package managers such as nlohmann-json or json-cpp.

I'd like to be able to run coverage studies with more than 30 transmitter sites. Is there a reason why the 30 transmitter limit is still necessary now that computers have larger RAM?

The documentation says it's supposed to be 4 for coverage map studies, but that no longer seems to be enforced now that this line is no longer there:

https://github.com/jmcmellen/splat/blob/08f06f56470a8361a5609503a6b1cb02f07f48dc/splat.cpp#L8032

Either way, the 30 site limit seems kind of arbitrary, and if I'd think if I'm individually passing QTH files to it on the command line, that as a user, if I specify a lot of them it takes a long time to calculate, it comes with the territory.

It would be great if we could read a list of TX and RX locations from a CSV file instead of having to specify them as individual files on the command line.

@hoche did some work before @watkipet and me started development in his unpublished 1.4.3 version. This includes:

• antenna height changes
• antenna azimuth and elevation reading
• ITM instead of ITWOM used
• DEM dynamic memory allocation
• ...

It would be useful if we can merge those changes into the refactor branch. Changes are deep-going and must be merged carefully by hand.
@hoche: Do you plan to do the merging, as you know your changes best? Or should @watkipet and me go through it? I think it would be useful to wait for my pull request from the "json" branch, which introduces the geotiff and json output.

The MSI format is a common format for antenna patterns within a single ASCII text file. The format is all straight forward and could be easily integrated into SPLAT.

Information on format: http://radiomobile.pe1mew.nl/?The_program:Definitions:MSI
Free antenna pattern editor: https://www.wireless-planning.com/antenna-pattern-editor

https://www.researchgate.net/publication/261061432_Comparison_of_Longley-Rice_ITM_and_ITWOM_propagation_models_for_DTV_and_FM_broadcasting

It would be very convenient to generate a .pgw world file when a .png plot file is made.
I have to do this manually to import as a raster file into QGIS and the details of the .pgw keep changing as plot radius is changed.

@hoche: Do you consider the warnings "ignoring return value" of numerous fgets and fscanf calls as relevant to be fixed? Should we add the compiler flag -Wno-unused-variable to cmake or rather catch their return values? It is desirable to compile without warnings in order to not lose sight of more important warnings like #14.

Currently, splat.cpp contains four functions for writing raster map outputs:

• WriteImage
• WriteImageDBM
• WriteImageLR
• WriteImageSS

Most of their code is identical, just the details differ. It would make sense to combine those in one function. That would make it easier to include new image formats (like GeoTiff or World Files, see #8 ). edit: was no problem.

Much of the code uses char* with malloc/free and strcpy, strdup, etc for filenames and for reading parameters from files. This should all be converted to std::string.

Presently, Dem holds data, mask, and signal. Only data contains elevation data. mask and signal are the results of calculations or loading place markers, etc. Likewise, ElevationMap isn't just concerned with elevation--that's where all the calculations take place.

I propose the following:

1. Rename ElevationMap to Workspace
2. Pull mask and signal out into separate layers
3. Make MapComputation and PathComputation abstract base classes. Concrete subclasses of these can operate in the Workspace and output to a layer.

I'm hoping this will accomplish a few things:

1. Decouple the results of the computation from the elevation data
2. Allow for adding new computations
3. Prevent one computation from overwriting data from another (unless desired)

Something I still need to think about is that there are multiple Dems per Workspace (see sr.maxpages). All the Dems have the same resolution (see sr.ippd). This is fine. Likewise, there should still be a 1:1 correspondence between Dems and pieces of output layer data. However, a Computation doesn't care about pages. All it cares about is latitude, longitude, and its output layer.

So I'm thinking that a Workspace defines the number of pages, and the resolution of each. Then layers can be added to the Workspace so long as each layer exposes the appropriate number of data pages each with the appropriate resolution.

It would be nice if SDF, SRTMGL, etc. were also just layers which provide elevation data. That would fulfill a desire I saw elsewhere to support loading elevation data from more than one source (i.e. high resolution data from one source, and low resolution data from another). If the elevation data sources are in layers, the Workspace can get data from whatever the top layer is. If there are holes in the top layer, the Workspace can get data from the next layer down. If a layer isn't at the appropriate resolution, it's responsible for interpolating. The bottom layer is always present, and it's sea-level.

By retaining the use of pages, each layer can perform sparse operations. If the Workspace asks an SDF layer for page 4, it can load it from the disk and cache it in memory on the fly. If the Workspace asks an SRTGML layer for page 5, it can load that page from an HTTP source if so configured and cache it on disk, in memory, or both.

Likewise, if a computation wants to write to a certain latitude and longitude, the Layer can determine which page that is, and create only a single page of output data rather than allocating all the pages of output data at runtime for all the layers (like we do now).

It would be really handy if I could specify transmitter sites using absolute height data (which I already have) instead of having to find the average ground level of the site and subtract it to get an AGL value.

A useful improvement to the coverage plot optionfeature with multiple transmitters would be to be able to plot a map of best server.
I have modified Johns original code to add an extra field in the map array. When coverage -c is plotted, it recorded the transmitter number and for each transmitter updated this field if the new calculation had stronger signal. I then assigned a colour to the transmitter number and plotted.

This task is to add CMake support to the refactor branch (or a branch off of that one).

I'm glad to see it was implemented, but I had to dig through the code to figure out how to use it. The old version relied on the binary's name, and that's still how it's shown in the docs.

Since the advent of the CMake build system (#4), some of the existing files are obsolete:

1. vstudio - Can now be generated using cmake -G "Visual Studio 15 2017 Win64"
2. install - After running cmake, you can now just run make install
3. README - Any reason this information can't be merged into README.md?
4. Makefile - Now generated by cmake
5. utils\Makefile - Now generated by cmake
6. utils\install - After running cmake, you can now just run make install
7. Subfolders under docs\* - The cmake build system automatically generates the .txt, .pdf, .ps and man-page outputs from splat.man

Any reason to keep what I've listed above? Is there anything else I should remove?

Right now there's a bunch of different functions where XML is generated by doing sprintfs. Much is redundant and could be combined together. However, it occurs to me that it might be better to incorporate some sort of template engine for that. I'm not talking about sucking in a full-bore templating engine like Jinja or Jade (wrong language anyway), just creating a simple substitution engine where key tags in the text file get replaced with something out of std::unordered_map.

I'll have to analyze the code some, but I don't think we need much more than that.

If we have complex data it'd get complicated, but even then we could suck in something like https://github.com/pantor/inja.

Some of the LRP parameters can be set or overridden by command line switches, but it would be nice if they could all be specified on the command line so the need for an LRP file could be eliminated.

Make logging more configurable, rather than write our own, investigate the use of spdlog

The present command-line parsing is difficult to understand and brittle. It's also subject to all sorts of buffer-overrun attacks (bad news for people putting a web interface in front of Splat! who don't sanitize their input). I suggest using a the 3rd party library for parsing command line arguments. The ones that seem reasonable (in order of what I think I'd prefer) are:

1. CLI11
2. gflags
3. tclap

Any concerns about using the 3rd party library? Any preference for one of these (or another)?

I'm still wondering if the SPLAT data files (*.sdf) are really necessary. Wouldn't it be possible to load a digital elevation model (DEM) into the RAM on the fly just after calculating the geographical extent?
In any case, it would be very useful to read geotiffs or other DEM file formats supported by GDAL directly without invoking srtm2sdf.
As a side note: For OpenTopoMap I prefer to have a global single geotif file, that is internally tiled. Selection of some extents is very fast.