Rather than a complete reversion, would it be possible to not apply the remove_nonprint function to affected models or, better yet, make sure the command termination character is preserved? Mike's commit references issue #1652 so it seems there is a reason Mike added this but may have inadvertently stripped the command terminator as well.

Rather than a complete reversion, would it be possible to not apply the remove_nonprint function to affected models or, better yet, make sure the command termination character is preserved?

Maybe! This revert will at least resolve the problem for people trying to use hamlib with impacted devices. We can certainly choose not to merge it, but now at least I can point people here who run into the same problem.

To be clear: at the moment, hamlib is now completely broken on a variety of Kenwood devices. I'm sure it's possible to create a more nuanced solution to #1652, but that will take time and given severe nature of the problem I wanted to make this fix available.

Mike's commit references issue #1652 so it seems there is a reason Mike added this but may have inadvertently stripped the command terminator as well.

The change was made in the context of Kenwood devices that use ; as the command terminator; it was never tested on devices that use \r.

How about checking the expected command terminator(kenwood_priv_caps.cmdtrm) - if it is printable, remove the nonprintables. If it isn't printable, then don't.

George, that's a much more elegant solution than my example in #1767 which only tests for \r and is hard coded.

Something like this -

diff --git a/rigs/kenwood/kenwood.c b/rigs/kenwood/kenwood.c
index 1596c2dfe..88655f2dd 100644
--- a/rigs/kenwood/kenwood.c
+++ b/rigs/kenwood/kenwood.c
@@ -445,7 +445,13 @@ transaction_read:
 
     // this fixes the case when some corrupt data is returned
     // let's us be a little more robust about funky serial data
-    remove_nonprint(buffer);
+    // If the terminator is printable(usually ';'), then there should be
+    //   no nonprintables in the message; if it isn't(usually '\r') then
+    //   don't touch the message
+    if (isprint(caps->cmdtrm))
+    {
+        remove_nonprint(buffer);
+    }
 
     if (retval < 0)
     {

George, check my proposed fix in #1767. The problem is outlined in #1652 and I think needs to be dealt with in the remove_nonprint function.

Then again, yours might be correct if radios that use EOM_TH (\r) need to pass unprintable characters to the host.

Lars, give George's patch preference over mine as it's likely correct in that radios that use EOM_TH should not have the message modified.

Since the previous commit works for some devices but breaks others, maybe instead of reverting it entirely, we could keep the old behavior behind a ./configure flag (e.g., --enable-removenonprint or similar).

This way, users who rely on the previous behavior can enable it explicitly, while the default remains compatible with the majority of devices.

Hi @brightkill

I think George's submission will work for both cases. Having to invoke configure time options for relatively common devices is not something we wish to do. This will be resolved in the library.

One question I have for you related to issue #1652 is whether the device firmware was updated by the authors to remove the unprintable characters?

@GeoBaltz I've updated this PR to adopt your proposed solution.

Thanks, Lars and George.

This will be in tomorrow's daily snapshot. I will also cherry-pick this over into the Hamlib-4.6.3 branch for a bug fix 4.6.4 release in a few weeks.