v1.10 - added simple check for GUI<->MWii communicati

[Hamburger]

Hi,
each time we add another parameter that gets transferred between GUI and MultiWii, the corresponding code to use/display this data has to be implemented in both GUI code and MWii code. Additionally, the transfer between GUI and MWii for this extra datum has to be implemented. For now this requires tweaking counters/constants in various locations, again both in GUI code and MWii code.

I for one have made errors more than once on these occasions and those are tedious to hunt down.
That was the reason why I started to implement two things
1. use #defines to denominate constants like NUMCHECKBOXES (if you did add another checkbox, you had to hunt down lots of code to adjust for transmission and computation)
2. for GUI-MWii communications added a control char to mark end of transmission. I did this for the W-command (transfer values from GUI to MWii into eeprom). Also I added an alarm on error of transmission on the MWii side.

Why do I brag about this?
Because I want you to participate in using this pattern throughout future code. If you are against it, then I am eager to hear that too.
Code is as always, on google rep in the trunk, *_shared , r379.

Cheers, Hamburger

[Niclas Hedlund]

Good idea's!

I have made some changes myself. (but not commited them yeat)
I added support for closing the com-port when pressing the "stop"-button. Then open the selected port again when pressing "start"-button.

This is due to I have the FTDI-chip onboard my MultiWii-controller board, so I need to restart the MultiWiiConfig application each time I unplug the USB-cable,
same when using Bluetooth link to the board and disconnecting the battery.

What do you think about this?
Should I commit the changes?

[Hamburger]

...same when using Bluetooth link to the board and disconnecting the battery.

Hm, not sure I understand your intentions regarding BT. For me the BT connection is established when I first start the application and it gets terminated upon closing the GUI. As this takes some time (and sometimes twiddling permissions before possible open again) I would not see any benefit in those many open/close connections?
May be selecting the port could be turned into a toggle like function?

What do you think about this?
Should I commit the changes?

yes, at least make it publicily available in the developers' branches tree.

[timecop]

pressing [stop] stops the GUI from repeatedly sending the "M" command right?
but you can still do other things like configure gains and press 'write' or calibrate accel/etc.
So stop does not mean 'close com port'. It just means "stop spamming the graph with stuff I don't care about while I do something else". So making it close com port is a little counterintuitive. Perhaps a "open/close" port button on the left tab where the com port listing is would make sense, including current status of whether its 'connected' or not. As there's currently no way to actually confirm this.

[frank26080115]

The right way to approach this problem is to write a common header file that is read by both the GUI source code and the AVR source code, so that the size of the data structure is automatically determined. But this really isn't practical when the GUI is written in Java and the firmware is in C.

Another way is to embed the expected size of the communication into the stream itself, which eliminates the need for the constants

Marking the end of a transmission with a magic number is not a good idea, what if your data represents the same number? Avoid magic numbers unless you are using human readable strings, in which case punctuation, control characters, and null termination becomes useful.

[Hamburger]

while I believe to understabd your reasoning I am not convinced it properly addresses the issue we face here.
Example: on one side make array sized 26, fill with comman char + values, repeat 26 send command.
On other side command char case - wait for 25 values, then process properly.

In an ideal world I would not want the developer to have to manualy count bytes to dereive the constant 26;
Also dealing with correspondent data layout in two languages is suboptimal.
If you have a working suggestion for the case as is, you are welcome

[frank26080115]

please see

http://code.google.com/p/palmquad/sourc ... Serial.pde
line 116

and then see

http://code.google.com/p/palmquad/sourc ... lmQuad.pde
line 553

notice how I keep track of how many bytes I'm sending automatically

[Hamburger]

looking good!
Will you submit the changes for read and write?

[ziss_dm]

Hi,
But maybe it is better to "inherit" the wheel instead inventing one?
http://www.qgroundcontrol.org/mavlink/start
http://code.google.com/p/copter-gcs/dow ... p&can=2&q=

regards,
ziss_dm

[frank26080115]

Hamburger, anybody is certainly free to adapt the changes I made, but the file I've just shown you cannot be plugged back into the original MultiWii code because of the extensive customizations I've done, if you want the same feature in MultiWii, it's going to have to be rewritten slightly again.

ziss_dm, that looks cool but it seems to be overkill in complexity in comparison to what has already been done.

[ziss_dm]

Hi,

Not sure about complexity though (atleast at MCU side):
sending:

Code: Select all

   // Initialize the required buffers 
   mavlink_message_t msg; 
   uint8_t buf[MAVLINK_MAX_PACKET_LEN];
...
   mavlink_msg_raw_imu_pack(100, 200, &msg, 0, xacc, yacc, zacc, xgyro, ygyro, zgyro, xmag, ymag, zmag);
   uint16_t len = mavlink_msg_to_send_buffer(buf, &msg);
   Serial.write(buf, len); 

Which is pretty much what we have now, except serialization is automatically generated.

message handeling:

Code: Select all

void comm_receive() { 
   mavlink_message_t msg; 
   mavlink_status_t status;
   //receive data over serial 
   while(Serial.available() > 0) { 
      uint8_t c = Serial.read();
      //try to get a new message 
      if(mavlink_parse_char(MAVLINK_COMM_0, c, &msg, &status)) { 
         // Handle message
          switch(msg.msgid) {
                 case MAVLINK_MSG_ID_SET_MODE: {
                    // set mode
                 }
                 break;
                 case MAVLINK_MSG_ID_ACTION:
               // EXECUTE ACTION
            break;
            default:
               //Do nothing
            break;
         }
      } 
      // And get the next one
   }
}

regards,
ziss_dm

[Hamburger]

Gentlemen,
so the big question is - what do we do (and who's gonna do it)?
I propose two steps (pending Alex' consent to integrate into dev version)
1. for this version the easy path: replace my atempt of stop control char with a checksum and test against that implementation
2. for next major release: check options like mavlink and compatiibility with existing ground control stations and protocols.

Consensus?
Alex?