From 65a13dfadb41f3ba957c90358bc953f1e654d34f Mon Sep 17 00:00:00 2001 From: David Ellis Date: Thu, 22 Feb 2024 09:51:56 -0600 Subject: [PATCH 1/8] Implement the cellToBoundary command for the h3 binary --- src/apps/filters/h3.c | 50 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/src/apps/filters/h3.c b/src/apps/filters/h3.c index 5625bc069..a8e0ec025 100644 --- a/src/apps/filters/h3.c +++ b/src/apps/filters/h3.c @@ -1,5 +1,5 @@ /* - * Copyright 2021 Uber Technologies, Inc. + * Copyright 2021, 2024 Uber Technologies, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -53,7 +53,10 @@ bool cellToLatLngCmd(int argc, char *argv[]) { return helpArg.found; } LatLng ll; - H3_EXPORT(cellToLatLng)(cell, &ll); + int err = H3_EXPORT(cellToLatLng)(cell, &ll); + if (err) { + return false; + } printf("%.10lf, %.10lf\n", H3_EXPORT(radsToDegs)(ll.lat), H3_EXPORT(radsToDegs)(ll.lng)); return true; @@ -110,6 +113,38 @@ bool latLngToCellCmd(int argc, char *argv[]) { return true; } +bool cellToBoundaryCmd(int argc, char* argv[]) { + Arg cellToBoundaryArg = { + .names = {"cellToBoundary"}, + .required = true, + .helpText = "Convert an H3 cell to an array of latitude/longitude coordinates defining its boundary", + }; + Arg helpArg = ARG_HELP; + DEFINE_CELL_ARG(cell, cellArg); + Arg *args[] = {&cellToBoundaryArg, &helpArg, &cellArg}; + if (parseArgs(argc, argv, 3, args, &helpArg, + "Convert an H3 cell to an array of latitude/longitude coordinates defining its boundary")) { + return helpArg.found; + } + CellBoundary cb; + int err = H3_EXPORT(cellToBoundary)(cell, &cb); + if (err) { + return false; + } + // TODO: Is it better to use brackets or parentheses for the output? + printf("[\n"); + for (int i = 0; i < cb.numVerts; i++) { + LatLng *ll = &cb.verts[i]; + printf( + " [%.10lf, %.10lf],\n", + H3_EXPORT(radsToDegs)(ll->lat), + H3_EXPORT(radsToDegs)(ll->lng) + ); + } + printf("]\n"); + return true; +} + bool generalHelp(int argc, char *argv[]) { Arg helpArg = ARG_HELP; Arg cellToLatLngArg = { @@ -121,12 +156,16 @@ bool generalHelp(int argc, char *argv[]) { .helpText = "Convert degrees latitude/longitude coordinate to an H3 cell.", }; - Arg *args[] = {&helpArg, &cellToLatLngArg, &latLngToCellArg}; + Arg cellToBoundaryArg = { + .names = {"cellToBoundary"}, + .helpText = "Convert an H3 cell to an array of latitude/longitude coordinates defining its boundary", + }; + Arg *args[] = {&helpArg, &cellToLatLngArg, &latLngToCellArg, &cellToBoundaryArg}; const char *helpText = "Please use one of the subcommands listed to perform an H3 " "calculation. Use h3 --help for details on the usage of " "any subcommand."; - return parseArgs(argc, argv, 3, args, &helpArg, helpText); + return parseArgs(argc, argv, 4, args, &helpArg, helpText); } int main(int argc, char *argv[]) { @@ -140,6 +179,9 @@ int main(int argc, char *argv[]) { if (has("latLngToCell", 1, argv) && latLngToCellCmd(argc, argv)) { return 0; } + if (has("cellToBoundary", 1, argv) && cellToBoundaryCmd(argc, argv)) { + return 0; + } if (generalHelp(argc, argv)) { return 0; } From 521f667f003f1c7cb1dc7e7526a43d552cc23921 Mon Sep 17 00:00:00 2001 From: David Ellis Date: Thu, 22 Feb 2024 10:17:25 -0600 Subject: [PATCH 2/8] Fix formatting --- src/apps/filters/h3.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/apps/filters/h3.c b/src/apps/filters/h3.c index a8e0ec025..38ac2b793 100644 --- a/src/apps/filters/h3.c +++ b/src/apps/filters/h3.c @@ -113,17 +113,20 @@ bool latLngToCellCmd(int argc, char *argv[]) { return true; } -bool cellToBoundaryCmd(int argc, char* argv[]) { +bool cellToBoundaryCmd(int argc, char *argv[]) { Arg cellToBoundaryArg = { .names = {"cellToBoundary"}, .required = true, - .helpText = "Convert an H3 cell to an array of latitude/longitude coordinates defining its boundary", + .helpText = + "Convert an H3 cell to an array of latitude/longitude coordinates " + "defining its boundary", }; Arg helpArg = ARG_HELP; DEFINE_CELL_ARG(cell, cellArg); Arg *args[] = {&cellToBoundaryArg, &helpArg, &cellArg}; if (parseArgs(argc, argv, 3, args, &helpArg, - "Convert an H3 cell to an array of latitude/longitude coordinates defining its boundary")) { + "Convert an H3 cell to an array of latitude/longitude " + "coordinates defining its boundary")) { return helpArg.found; } CellBoundary cb; @@ -135,11 +138,8 @@ bool cellToBoundaryCmd(int argc, char* argv[]) { printf("[\n"); for (int i = 0; i < cb.numVerts; i++) { LatLng *ll = &cb.verts[i]; - printf( - " [%.10lf, %.10lf],\n", - H3_EXPORT(radsToDegs)(ll->lat), - H3_EXPORT(radsToDegs)(ll->lng) - ); + printf(" [%.10lf, %.10lf],\n", H3_EXPORT(radsToDegs)(ll->lat), + H3_EXPORT(radsToDegs)(ll->lng)); } printf("]\n"); return true; @@ -158,9 +158,12 @@ bool generalHelp(int argc, char *argv[]) { }; Arg cellToBoundaryArg = { .names = {"cellToBoundary"}, - .helpText = "Convert an H3 cell to an array of latitude/longitude coordinates defining its boundary", + .helpText = + "Convert an H3 cell to an array of latitude/longitude coordinates " + "defining its boundary", }; - Arg *args[] = {&helpArg, &cellToLatLngArg, &latLngToCellArg, &cellToBoundaryArg}; + Arg *args[] = {&helpArg, &cellToLatLngArg, &latLngToCellArg, + &cellToBoundaryArg}; const char *helpText = "Please use one of the subcommands listed to perform an H3 " "calculation. Use h3 --help for details on the usage of " From 1c637945f12bb34f725e6bc5e432540b5f9b6ec7 Mon Sep 17 00:00:00 2001 From: David Ellis Date: Thu, 22 Feb 2024 10:34:27 -0600 Subject: [PATCH 3/8] Fix error type Co-authored-by: Isaac Brodsky --- src/apps/filters/h3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/apps/filters/h3.c b/src/apps/filters/h3.c index 38ac2b793..139df8c76 100644 --- a/src/apps/filters/h3.c +++ b/src/apps/filters/h3.c @@ -53,7 +53,7 @@ bool cellToLatLngCmd(int argc, char *argv[]) { return helpArg.found; } LatLng ll; - int err = H3_EXPORT(cellToLatLng)(cell, &ll); + H3Error err = H3_EXPORT(cellToLatLng)(cell, &ll); if (err) { return false; } @@ -130,7 +130,7 @@ bool cellToBoundaryCmd(int argc, char *argv[]) { return helpArg.found; } CellBoundary cb; - int err = H3_EXPORT(cellToBoundary)(cell, &cb); + H3Error err = H3_EXPORT(cellToBoundary)(cell, &cb); if (err) { return false; } From 0b761dd01e7bcf33cabb274820c244547aebf461 Mon Sep 17 00:00:00 2001 From: David Ellis Date: Thu, 22 Feb 2024 10:40:20 -0600 Subject: [PATCH 4/8] Define a CONSTANT for the help's arg length --- src/apps/filters/h3.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/apps/filters/h3.c b/src/apps/filters/h3.c index 139df8c76..e66fcd3d1 100644 --- a/src/apps/filters/h3.c +++ b/src/apps/filters/h3.c @@ -164,11 +164,13 @@ bool generalHelp(int argc, char *argv[]) { }; Arg *args[] = {&helpArg, &cellToLatLngArg, &latLngToCellArg, &cellToBoundaryArg}; + #define ARGLEN 4 + const char *helpText = "Please use one of the subcommands listed to perform an H3 " "calculation. Use h3 --help for details on the usage of " "any subcommand."; - return parseArgs(argc, argv, 4, args, &helpArg, helpText); + return parseArgs(argc, argv, ARGLEN, args, &helpArg, helpText); } int main(int argc, char *argv[]) { From 4100248b434257cb37a031ad5df9c075de289050 Mon Sep 17 00:00:00 2001 From: David Ellis Date: Thu, 22 Feb 2024 10:46:52 -0600 Subject: [PATCH 5/8] Fix formatting --- src/apps/filters/h3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apps/filters/h3.c b/src/apps/filters/h3.c index e66fcd3d1..57cc36c95 100644 --- a/src/apps/filters/h3.c +++ b/src/apps/filters/h3.c @@ -164,7 +164,7 @@ bool generalHelp(int argc, char *argv[]) { }; Arg *args[] = {&helpArg, &cellToLatLngArg, &latLngToCellArg, &cellToBoundaryArg}; - #define ARGLEN 4 +#define ARGLEN 4 const char *helpText = "Please use one of the subcommands listed to perform an H3 " From 4f5b7c96039302c1cde4b850b85495a8940de552 Mon Sep 17 00:00:00 2001 From: David Ellis Date: Sat, 23 Mar 2024 20:02:07 -0500 Subject: [PATCH 6/8] Add the test for the cellToBoundary sub-command, convert output to WKT where applicable, and eliminate hardwired constants in favor of sizeof trick @sahrk mentioned --- CMakeLists.txt | 5 +++-- CMakeTests.cmake | 5 +++-- src/apps/filters/h3.c | 26 ++++++++++++++++---------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4c90917a1..e7893ef36 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -180,8 +180,9 @@ set(EXAMPLE_SOURCE_FILES examples/neighbors.c examples/compactCells.c examples/edge.c) +set(H3_BIN_SOURCE_FILES + src/apps/filters/h3.c) set(OTHER_SOURCE_FILES - src/apps/filters/h3.c src/apps/filters/cellToLatLng.c src/apps/filters/cellToLocalIj.c src/apps/filters/localIjToCell.c @@ -289,7 +290,7 @@ set(OTHER_SOURCE_FILES src/apps/benchmarks/benchmarkH3Api.c) set(ALL_SOURCE_FILES - ${LIB_SOURCE_FILES} ${APP_SOURCE_FILES} ${TEST_APP_SOURCE_FILES} ${OTHER_SOURCE_FILES}) + ${LIB_SOURCE_FILES} ${APP_SOURCE_FILES} ${TEST_APP_SOURCE_FILES} ${H3_BIN_SOURCE_FILES} ${OTHER_SOURCE_FILES}) # This is done for quality control purposes (to detect if any source files are missing from # our list), but is not done as the authoritative list as per CMake developer recommendations. diff --git a/CMakeTests.cmake b/CMakeTests.cmake index a5d57e279..0f585489a 100644 --- a/CMakeTests.cmake +++ b/CMakeTests.cmake @@ -1,4 +1,4 @@ -# Copyright 2017-2022 Uber Technologies, Inc. +# Copyright 2017-2022, 2024 Uber Technologies, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -228,8 +228,9 @@ add_h3_test(testGridDistanceExhaustive src/apps/testapps/testGridDistanceExhaust add_h3_test(testH3CellAreaExhaustive src/apps/testapps/testH3CellAreaExhaustive.c) add_h3_test(testCellToBBoxExhaustive src/apps/testapps/testCellToBBoxExhaustive.c) -add_h3_cli_test(testCliCellToLatLng "cellToLatLng -c 8928342e20fffff" "37.5012466151, -122.5003039349") +add_h3_cli_test(testCliCellToLatLng "cellToLatLng -c 8928342e20fffff" "POINT(37.5012466151 -122.5003039349)") add_h3_cli_test(testCliLatLngToCell "latLngToCell --lat 20 --lng 123 -r 2" "824b9ffffffffff") +add_h3_cli_test(testCliCellToBoundary "cellToBoundary -c 8928342e20fffff" "POLYGON((37.4997389893 -122.4990471431, 37.5014245698 -122.4979805011, 37.5029321860 -122.4992373065, 37.5027541980 -122.5015607527, 37.5010686174 -122.5026273256, 37.4995610248 -122.5013705214, 37.4997389893 -122.4990471431))") if(BUILD_ALLOC_TESTS) add_h3_library(h3WithTestAllocators test_prefix_) diff --git a/src/apps/filters/h3.c b/src/apps/filters/h3.c index 57cc36c95..2fad62ed4 100644 --- a/src/apps/filters/h3.c +++ b/src/apps/filters/h3.c @@ -48,7 +48,7 @@ bool cellToLatLngCmd(int argc, char *argv[]) { Arg helpArg = ARG_HELP; DEFINE_CELL_ARG(cell, cellArg); Arg *args[] = {&cellToLatLngArg, &helpArg, &cellArg}; - if (parseArgs(argc, argv, 3, args, &helpArg, + if (parseArgs(argc, argv, sizeof(args) / sizeof(Arg *), args, &helpArg, "Convert an H3 cell to a latitude/longitude coordinate")) { return helpArg.found; } @@ -57,7 +57,9 @@ bool cellToLatLngCmd(int argc, char *argv[]) { if (err) { return false; } - printf("%.10lf, %.10lf\n", H3_EXPORT(radsToDegs)(ll.lat), + // Using WKT formatting for the output. TODO: Add support for JSON + // formatting + printf("POINT(%.10lf %.10lf)\n", H3_EXPORT(radsToDegs)(ll.lat), H3_EXPORT(radsToDegs)(ll.lng)); return true; } @@ -95,7 +97,7 @@ bool latLngToCellCmd(int argc, char *argv[]) { Arg *args[] = {&latLngToCellArg, &helpArg, &resArg, &latArg, &lngArg}; if (parseArgs( - argc, argv, 5, args, &helpArg, + argc, argv, sizeof(args) / sizeof(Arg *), args, &helpArg, "Convert degrees latitude/longitude coordinate to an H3 cell.")) { return helpArg.found; } @@ -105,6 +107,7 @@ bool latLngToCellCmd(int argc, char *argv[]) { H3Index c; H3Error e = H3_EXPORT(latLngToCell)(&ll, res, &c); + // TODO: Add support for JSON formatting if (e == E_SUCCESS) { h3Println(c); } else { @@ -124,7 +127,7 @@ bool cellToBoundaryCmd(int argc, char *argv[]) { Arg helpArg = ARG_HELP; DEFINE_CELL_ARG(cell, cellArg); Arg *args[] = {&cellToBoundaryArg, &helpArg, &cellArg}; - if (parseArgs(argc, argv, 3, args, &helpArg, + if (parseArgs(argc, argv, sizeof(args) / sizeof(Arg *), args, &helpArg, "Convert an H3 cell to an array of latitude/longitude " "coordinates defining its boundary")) { return helpArg.found; @@ -134,14 +137,17 @@ bool cellToBoundaryCmd(int argc, char *argv[]) { if (err) { return false; } - // TODO: Is it better to use brackets or parentheses for the output? - printf("[\n"); + // Using WKT formatting for the output. TODO: Add support for JSON + // formatting + printf("POLYGON(("); for (int i = 0; i < cb.numVerts; i++) { LatLng *ll = &cb.verts[i]; - printf(" [%.10lf, %.10lf],\n", H3_EXPORT(radsToDegs)(ll->lat), + printf("%.10lf %.10lf, ", H3_EXPORT(radsToDegs)(ll->lat), H3_EXPORT(radsToDegs)(ll->lng)); } - printf("]\n"); + // WKT has the first and last points match, so re-print the first one + printf("%.10lf %.10lf))\n", H3_EXPORT(radsToDegs)(cb.verts[0].lat), + H3_EXPORT(radsToDegs)(cb.verts[0].lng)); return true; } @@ -164,13 +170,13 @@ bool generalHelp(int argc, char *argv[]) { }; Arg *args[] = {&helpArg, &cellToLatLngArg, &latLngToCellArg, &cellToBoundaryArg}; -#define ARGLEN 4 const char *helpText = "Please use one of the subcommands listed to perform an H3 " "calculation. Use h3 --help for details on the usage of " "any subcommand."; - return parseArgs(argc, argv, ARGLEN, args, &helpArg, helpText); + return parseArgs(argc, argv, sizeof(args) / sizeof(Arg *), args, &helpArg, + helpText); } int main(int argc, char *argv[]) { From 0447a2b8dd52e78cae23caf6bd8f52cc395475f0 Mon Sep 17 00:00:00 2001 From: David Ellis Date: Sun, 24 Mar 2024 10:30:06 -0500 Subject: [PATCH 7/8] Switch to the 'wrong' (lng, lat) ordering ;) --- CMakeTests.cmake | 4 ++-- src/apps/filters/h3.c | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CMakeTests.cmake b/CMakeTests.cmake index 0f585489a..a84f1b0d4 100644 --- a/CMakeTests.cmake +++ b/CMakeTests.cmake @@ -228,9 +228,9 @@ add_h3_test(testGridDistanceExhaustive src/apps/testapps/testGridDistanceExhaust add_h3_test(testH3CellAreaExhaustive src/apps/testapps/testH3CellAreaExhaustive.c) add_h3_test(testCellToBBoxExhaustive src/apps/testapps/testCellToBBoxExhaustive.c) -add_h3_cli_test(testCliCellToLatLng "cellToLatLng -c 8928342e20fffff" "POINT(37.5012466151 -122.5003039349)") +add_h3_cli_test(testCliCellToLatLng "cellToLatLng -c 8928342e20fffff" "POINT(-122.5003039349 37.5012466151)") add_h3_cli_test(testCliLatLngToCell "latLngToCell --lat 20 --lng 123 -r 2" "824b9ffffffffff") -add_h3_cli_test(testCliCellToBoundary "cellToBoundary -c 8928342e20fffff" "POLYGON((37.4997389893 -122.4990471431, 37.5014245698 -122.4979805011, 37.5029321860 -122.4992373065, 37.5027541980 -122.5015607527, 37.5010686174 -122.5026273256, 37.4995610248 -122.5013705214, 37.4997389893 -122.4990471431))") +add_h3_cli_test(testCliCellToBoundary "cellToBoundary -c 8928342e20fffff" "POLYGON((-122.4990471431 37.4997389893, -122.4979805011 37.5014245698, -122.4992373065 37.5029321860, -122.5015607527 37.5027541980, -122.5026273256 37.5010686174, -122.5013705214 37.4995610248, -122.4990471431 37.4997389893))") if(BUILD_ALLOC_TESTS) add_h3_library(h3WithTestAllocators test_prefix_) diff --git a/src/apps/filters/h3.c b/src/apps/filters/h3.c index 2fad62ed4..ccba9346e 100644 --- a/src/apps/filters/h3.c +++ b/src/apps/filters/h3.c @@ -59,8 +59,8 @@ bool cellToLatLngCmd(int argc, char *argv[]) { } // Using WKT formatting for the output. TODO: Add support for JSON // formatting - printf("POINT(%.10lf %.10lf)\n", H3_EXPORT(radsToDegs)(ll.lat), - H3_EXPORT(radsToDegs)(ll.lng)); + printf("POINT(%.10lf %.10lf)\n", H3_EXPORT(radsToDegs)(ll.lng), + H3_EXPORT(radsToDegs)(ll.lat)); return true; } @@ -142,12 +142,12 @@ bool cellToBoundaryCmd(int argc, char *argv[]) { printf("POLYGON(("); for (int i = 0; i < cb.numVerts; i++) { LatLng *ll = &cb.verts[i]; - printf("%.10lf %.10lf, ", H3_EXPORT(radsToDegs)(ll->lat), - H3_EXPORT(radsToDegs)(ll->lng)); + printf("%.10lf %.10lf, ", H3_EXPORT(radsToDegs)(ll->lng), + H3_EXPORT(radsToDegs)(ll->lat)); } // WKT has the first and last points match, so re-print the first one - printf("%.10lf %.10lf))\n", H3_EXPORT(radsToDegs)(cb.verts[0].lat), - H3_EXPORT(radsToDegs)(cb.verts[0].lng)); + printf("%.10lf %.10lf))\n", H3_EXPORT(radsToDegs)(cb.verts[0].lng), + H3_EXPORT(radsToDegs)(cb.verts[0].lat)); return true; } From 9d416a435afc8c55d44998a931e763c77e3a1350 Mon Sep 17 00:00:00 2001 From: David Ellis Date: Mon, 25 Mar 2024 17:41:15 -0500 Subject: [PATCH 8/8] Update help text --- src/apps/filters/h3.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/apps/filters/h3.c b/src/apps/filters/h3.c index ccba9346e..9734b5d36 100644 --- a/src/apps/filters/h3.c +++ b/src/apps/filters/h3.c @@ -43,13 +43,13 @@ bool cellToLatLngCmd(int argc, char *argv[]) { Arg cellToLatLngArg = { .names = {"cellToLatLng"}, .required = true, - .helpText = "Convert an H3 cell to a latitude/longitude coordinate", + .helpText = "Convert an H3 cell to a WKT POINT coordinate", }; Arg helpArg = ARG_HELP; DEFINE_CELL_ARG(cell, cellArg); Arg *args[] = {&cellToLatLngArg, &helpArg, &cellArg}; if (parseArgs(argc, argv, sizeof(args) / sizeof(Arg *), args, &helpArg, - "Convert an H3 cell to a latitude/longitude coordinate")) { + "Convert an H3 cell to a WKT POINT coordinate")) { return helpArg.found; } LatLng ll; @@ -120,16 +120,14 @@ bool cellToBoundaryCmd(int argc, char *argv[]) { Arg cellToBoundaryArg = { .names = {"cellToBoundary"}, .required = true, - .helpText = - "Convert an H3 cell to an array of latitude/longitude coordinates " - "defining its boundary", + .helpText = "Convert an H3 cell to a WKT POLYGON defining its boundary", }; Arg helpArg = ARG_HELP; DEFINE_CELL_ARG(cell, cellArg); Arg *args[] = {&cellToBoundaryArg, &helpArg, &cellArg}; - if (parseArgs(argc, argv, sizeof(args) / sizeof(Arg *), args, &helpArg, - "Convert an H3 cell to an array of latitude/longitude " - "coordinates defining its boundary")) { + if (parseArgs( + argc, argv, sizeof(args) / sizeof(Arg *), args, &helpArg, + "Convert an H3 cell to a WKT POLYGON defining its boundary")) { return helpArg.found; } CellBoundary cb; @@ -155,7 +153,7 @@ bool generalHelp(int argc, char *argv[]) { Arg helpArg = ARG_HELP; Arg cellToLatLngArg = { .names = {"cellToLatLng"}, - .helpText = "Convert an H3 cell to a latitude/longitude coordinate", + .helpText = "Convert an H3 cell to a WKT POINT coordinate", }; Arg latLngToCellArg = { .names = {"latLngToCell"}, @@ -164,9 +162,7 @@ bool generalHelp(int argc, char *argv[]) { }; Arg cellToBoundaryArg = { .names = {"cellToBoundary"}, - .helpText = - "Convert an H3 cell to an array of latitude/longitude coordinates " - "defining its boundary", + .helpText = "Convert an H3 cell to a WKT POLYGON defining its boundary", }; Arg *args[] = {&helpArg, &cellToLatLngArg, &latLngToCellArg, &cellToBoundaryArg};