thijse / Arduino-CmdMessenger

CmdMessenger Communication library for Arduino & .NET
Other
208 stars 87 forks source link

Reading a string messes up all the arguments. #4

Closed depau closed 8 years ago

depau commented 9 years ago

I've been using CmdMessenger for a while and it seemed to work, however my current application requires the reception of a string through serial. Here's the function:

void onBtChName() {
  sendAck(kACK, kBT_CHNAME, F("Storing new name."));
  uint8_t len = constrain(cmdMessenger.readInt16Arg(), 0, 30);
  cmdMessenger.copyStringArg(bt_newname, len+1);
  Serial.print("New name ");
  Serial.write(bt_newname, len);
  Serial.println(len);
  bt_newnamelen = len;
  bt_chname = true;
  sendAck(kDONE, kBT_CHNAME, F("New name stored. Please commit."));
}

This is the callback attached to kBT_CHNAME, which is command 15. I'd expect it to read two arguments, an integer and a string. However it does not work. If I comment out the string reading part, everything works fine (ex. if I send 15,12,34,56,78;, the len variable would contain 12 as I would expect). However, if I also read the string (which is odd, as the string is read after the integer), it skips two arguments. So while 15,9,Davideddu; does not work, 15,0,0,9,Davideddu; does. Another example, if I send 15,12,34,56,78; len would be set to 56 (actually 30 because of constrain, but it doesn't really matter) and bt_newname would contain 78 + 28 bytes of crap.

This is a pretty odd bug.

P.S.: I wrote a python port of CmdMessenger with a similar API that, however, tries to make use of Python-specific features. you can find it here. I'll document it and add a readme at some point.

thijse commented 9 years ago

Hmm, this is very strange. If you can me provide a compilable sketch that displays this behavior, I will check it out!

depau commented 9 years ago

This one: https://github.com/Davideddu/BluEOS-Arduino Should work on ATmega328 or any other mcu with more flash/sram. Uncomment the 2nd and the 3rd lines to enable the bluetooth API, then send

15,<length>,<string>;

for example

15,7,Arduino;

then try one of these instead:

15,0,0,7,Arduino
15,this isn't,being read,7,Arduino;

The first one will print this:

3,15,Storing new name.;
New name 0
12,15,New name stored. Please commit.;

However what it should print (which is what you get with the second command) is this:

3,15,Storing new name.;
New name Arduino7
12,15,New name stored. Please commit.;

If you run 14; (kBT_COMMIT) right after that, it should print AT+NAMEArduino (where Arduino is whatever string you sent earlier) then restart.

depau commented 9 years ago

P.S.: you need SoftReset. 15 is kBT_CHNAME which accepts two arguments, an integer (the length) and a string (the new name which will be sent to the bluetooth serial module.

valkuc commented 8 years ago

You have an error in your code in onBtChName method. Line: uint8_t len = constrain(cmdMessenger.readInt16Arg(), 0, 30); constrain is a macro: #define constrain(amt,low,high) ((amt)<(low)?(low):((amt)>(high)?(high):(amt))) so in your case "amt" parameter get expanded three times and when code is running "cmdMessenger.readInt16Arg()" got called three times...

To solve you problem just assign cmdMessenger call to temporary variable:

uint8_t len = cmdMessenger.readInt16Arg();
len = constrain(len, 0, 30);