From 571b894019a7f60d1d80c0d584196546bc953a2e Mon Sep 17 00:00:00 2001
From: Shahram Najm <masn@ecmwf.int>
Date: Fri, 22 Sep 2023 13:04:58 +0100
Subject: [PATCH] Add better error messages

---
 src/grib_accessor_class_gen.cc        | 10 ++++++++--
 src/grib_accessor_class_non_alpha.cc  |  7 ++-----
 src/grib_accessor_class_to_integer.cc |  7 ++-----
 tests/grib_ecc-1167.sh                | 10 ++++++++++
 4 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/src/grib_accessor_class_gen.cc b/src/grib_accessor_class_gen.cc
index bd7341160..75f84bebd 100644
--- a/src/grib_accessor_class_gen.cc
+++ b/src/grib_accessor_class_gen.cc
@@ -469,7 +469,10 @@ static int pack_long(grib_accessor* a, const long* v, size_t* len)
         grib_context_free(c, val);
         return ret;
     }
-    grib_context_log(c, GRIB_LOG_ERROR, "Should not grib_pack %s as long", a->name);
+    grib_context_log(c, GRIB_LOG_ERROR, "Should not pack '%s' as an integer", a->name);
+    if (a->cclass->pack_string && a->cclass->pack_string != &pack_string) {
+        grib_context_log(c, GRIB_LOG_ERROR, "Try packing as a string");
+    }
     return GRIB_NOT_IMPLEMENTED;
 }
 
@@ -507,7 +510,10 @@ static int pack_double(grib_accessor* a, const double* v, size_t* len)
     if (do_pack_as_long) {
         return pack_double_array_as_long(a, v, len);
     }
-    grib_context_log(c, GRIB_LOG_ERROR, "Should not grib_pack %s as double", a->name);
+    grib_context_log(c, GRIB_LOG_ERROR, "Should not pack '%s' as a double", a->name);
+    if (a->cclass->pack_string && a->cclass->pack_string != &pack_string) {
+        grib_context_log(c, GRIB_LOG_ERROR, "Try packing as a string");
+    }
     return GRIB_NOT_IMPLEMENTED;
 }
 
diff --git a/src/grib_accessor_class_non_alpha.cc b/src/grib_accessor_class_non_alpha.cc
index 3b5badd23..e0f52f4d9 100644
--- a/src/grib_accessor_class_non_alpha.cc
+++ b/src/grib_accessor_class_non_alpha.cc
@@ -7,9 +7,6 @@
  * In applying this licence, ECMWF does not waive the privileges and immunities granted to it by
  * virtue of its status as an intergovernmental organisation nor does it submit to any jurisdiction.
  */
-/***********************************************************
- *  Enrico Fucile
- ***********************************************************/
 
 #include "grib_api_internal.h"
 /*
@@ -181,13 +178,13 @@ static int pack_string(grib_accessor* a, const char* val, size_t* len)
 
 static int pack_long(grib_accessor* a, const long* v, size_t* len)
 {
-    grib_context_log(a->context, GRIB_LOG_ERROR, " Should not pack %s as long", a->name);
+    grib_context_log(a->context, GRIB_LOG_ERROR, "Should not pack %s as an integer", a->name);
     return GRIB_NOT_IMPLEMENTED;
 }
 
 static int pack_double(grib_accessor* a, const double* v, size_t* len)
 {
-    grib_context_log(a->context, GRIB_LOG_ERROR, " Should not pack %s  as double", a->name);
+    grib_context_log(a->context, GRIB_LOG_ERROR, "Should not pack %s as a double", a->name);
     return GRIB_NOT_IMPLEMENTED;
 }
 
diff --git a/src/grib_accessor_class_to_integer.cc b/src/grib_accessor_class_to_integer.cc
index a51ff6972..30fa00251 100644
--- a/src/grib_accessor_class_to_integer.cc
+++ b/src/grib_accessor_class_to_integer.cc
@@ -7,9 +7,6 @@
  * In applying this licence, ECMWF does not waive the privileges and immunities granted to it by
  * virtue of its status as an intergovernmental organisation nor does it submit to any jurisdiction.
  */
-/***********************************************************
- *  Enrico Fucile
- ***********************************************************/
 
 #include "grib_api_internal.h"
 /*
@@ -206,13 +203,13 @@ static int pack_string(grib_accessor* a, const char* val, size_t* len)
 
 static int pack_long(grib_accessor* a, const long* v, size_t* len)
 {
-    grib_context_log(a->context, GRIB_LOG_ERROR, " Should not pack %s as long", a->name);
+    grib_context_log(a->context, GRIB_LOG_ERROR, "Should not pack %s as an integer", a->name);
     return GRIB_NOT_IMPLEMENTED;
 }
 
 static int pack_double(grib_accessor* a, const double* v, size_t* len)
 {
-    grib_context_log(a->context, GRIB_LOG_ERROR, " Should not pack %s  as double", a->name);
+    grib_context_log(a->context, GRIB_LOG_ERROR, "Should not pack %s as a double", a->name);
     return GRIB_NOT_IMPLEMENTED;
 }
 
diff --git a/tests/grib_ecc-1167.sh b/tests/grib_ecc-1167.sh
index 3bd4cb37f..32db7af41 100755
--- a/tests/grib_ecc-1167.sh
+++ b/tests/grib_ecc-1167.sh
@@ -32,6 +32,7 @@ grib_check_key_equals $temp2 uuidOfHGrid '10991111111111111111115000110000'
 
 
 # Test errors
+# --------------
 set +e
 # Invalid HEX
 ${tools_dir}/grib_set -s uuidOfHGrid=DEZZBEef10203040b00b1e50001100FF $temp1 $temp2 2>$errlog
@@ -40,6 +41,15 @@ set -e
 [ $status -ne 0 ]
 grep -q "Invalid hex byte specfication 'ZZ'" $errlog
 
+# Packing as an integer
+set +e
+${tools_dir}/grib_set -s uuidOfHGrid=12345 $temp1 $temp2 2>$errlog
+status=$?
+set -e
+[ $status -ne 0 ]
+grep -q "Should not pack 'uuidOfHGrid' as an integer" $errlog
+grep -q "Try packing as a string" $errlog
+
 
 # Clean up
 rm -f $temp1 $temp2 $errlog