Possible bugfix: newlines not being sent

[dumpfheimer]

Hi!

While playing with your library, I often had issues with newlines not being sent. I was able to fix this problem: It seems like the input_string length is passed to the writeSerialPort function, disregarding the newline character.

Disclaimer: I did not spend a long time investigating this, and apparently it works without it for some people. But this did seem wrong to me, and now everything is working fine with my due.

Could you please look over the changes and let me know what you think?

[BitsAndDroids]

I'm currently looking into this! In theory, the line above should set the last character to /n all the time. c_string[input_string.size()] = '\n'; (access the array element at input_string.size() should always set the last character. The spacing for the extra character should already be set at the first line: auto *const c_string = new char[input_string.size() + 1]; So I'm a bit puzzled why your fix works but I'm on it.

[dumpfheimer]

Well, let's take this example:

void sendToArduino(float received, std::string prefix, const char *portName) {
  auto intVal = static_cast<int>(received);
  SerialPort arduino(portName);
  if (!arduino.isConnected()) {
    connectionError = true;
  } else {
    connectionError = false;
  }
  const auto value = intVal;
  auto input_string = prefix + std::to_string(value);
  auto *const c_string = new char[input_string.size() + 1];
  std::copy(input_string.begin(), input_string.end(), c_string);
  c_string[input_string.size()] = '\n';
  arduino.writeSerialPort(c_string, input_string.length() + 1);
  delete[] c_string;
}

To me it looks like the value is being preset with something (prefix) and then stored into c_string. c_string gehts the length of input_string + 1, which is good, because it gets the newline after that.

but arduino.writeSerialPort() then gets the input_string length, not the c_string length, which is missing the newline.

I believe this might be the root of the problem. So a nicer fix probably would be to use the length of c_string. But I was not sure how to do that.

Also, there is a mix of .size() and .length() - not quite sure if that could cause anything?

[BitsAndDroids]

Oh lord... When your nose is so close to the answer that you aren't able to spot it. Thanks a bunch. I went over it and over it and couldn't spot something basic as this. I will fix it with the length of the c_string to be concise. Thanks for the fresh pair of eyes

[dumpfheimer]

Sounds great, thanks!

[BitsAndDroids]

Perhaps a stupid question, but could you test if any inputs work with your DUE on the latest build? You're the only one I know that has a DUE besides me and Pontiac. It works for me but for him, it doesn't. Trying to rule out of it's an incident or a bigger issue. If you don't have time / don't want to, that is fine as well.

[dumpfheimer]

Its working inconsistently. If I quickly toggle the frequency in the FS sometimes I get an update, most of the time I do not, though.

[dumpfheimer]

I wrote a comment in your commit: https://github.com/BitsAndDroids/Bits-and-Droids-flight-connector/commit/cf1f8393c11130bf4f268357cd339e1f96568b8d#r48181759

[BitsAndDroids]

Thanks for checking. But you were able to send a command via the connector with a due?

Van: dumpfheimer @.> Verzonden: vrijdag 12 maart 2021 15:58 Aan: BitsAndDroids/Bits-and-Droids-flight-connector @.> CC: BitsAndDroids @.>; Comment @.> Onderwerp: Re: [BitsAndDroids/Bits-and-Droids-flight-connector] Possible bugfix: newlines not being sent (#1)

Its working inconsistently. If I quickly toggle the frequency in the FS sometimes I get an update, most of the time I do not, though.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/BitsAndDroids/Bits-and-Droids-flight-connector/pull/1#issuecomment-797542564 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ASRNKJZJSGPUJDLVCUEYQCDTDITXZANCNFSM4ZASE5IQ . https://github.com/notifications/beacon/ASRNKJ5INQEI75ZS7K7NASDTDITXZA5CNFSM4ZASE5I2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOF6EYRJA.gif