New Multiwii Serial Protocol

Tommie · Post by **Tommie** » Fri Jun 08, 2012 8:28 pm

No, the issue is the use of String. Doing stunts to find out the encoding does not make it better :-)

Since it's Java, there is no need to use Strings; a Array/Vector/List of Character/Byte/Integer is fine and has the advantage of being mutable (yes, you can use stringbuffer, but why?). I already fixed it :-)

Tommie · Post by **Tommie** » Fri Jun 08, 2012 8:48 pm

kos wrote:edit : using String the way it was proposed is ok , the way you dot it now i ok .. the only point is not calling serial.write() for individual char in a for .. it will slow the gui too much

It failed completely on my system; probably it's a locale issue; however, I took your point and changed the requestRoutine, so now the entire sequence of bytes[] is written as one. It's actually a noticable speedup, although Processing is still slow as hell.

EOSBandi · Post by **EOSBandi** » Fri Jun 08, 2012 8:52 pm

Tommie,

If you don't have gps to test then please keep your hands off from the gps code. For example the i2c_init_done had it purpose, it is supposed to avoid i2c errors at startup from the i2c gps.
And GPS_DistanceToHold and GPS_DirectiontoHold are just leftovers from old GPS code
EosBandi

Tommie · Post by **Tommie** » Fri Jun 08, 2012 9:03 pm

EOSBandi wrote:If you don't have gps to test then please keep your hands off from the gps code. For example the i2c_init_done had it purpose, it is supposed to avoid i2c errors at startup from the i2c gps.

But you are aware that the variable is not written anywhere?
And the function GPS_set_pids() (which is supposed to be guarded by this flag) is called during setup() no matter what?

EOSBandi · Post by **EOSBandi** » Fri Jun 08, 2012 9:10 pm

Tommie wrote:
EOSBandi wrote:If you don't have gps to test then please keep your hands off from the gps code. For example the i2c_init_done had it purpose, it is supposed to avoid i2c errors at startup from the i2c gps.

But you are aware that the variable is not written anywhere?
And the function GPS_set_pids() (which is supposed to be guarded by this flag) is called during setup() no matter what?

It seems it's a merge error. Is supposed to be set at the end of initSensors. And it is called setup AFTER initSensors. But EEPROMread is called BEFORE initSensors at setup, so callinf the GPS_set_pids() in the EEPROMread at startup will cause i2cerrors.

Tommie · Post by **Tommie** » Fri Jun 08, 2012 9:14 pm

Well, looks ike nobody noticed that after merging, so don't blame the messenger (for removing the variable).

copterrichie · Post by **copterrichie** » Fri Jun 08, 2012 9:23 pm

Guys, seems to me, we need to implement the CODE review procedure before code is committed to the main or shared libraries. Google Code does have that feature.

EOSBandi · Post by **EOSBandi** » Fri Jun 08, 2012 9:25 pm

Agree

Tommie · Post by **Tommie** » Fri Jun 08, 2012 9:30 pm

Reviewing the submitted code is not the problem; the problem at hand is merging. That's the major issue regarding centralized version control system in contrast to distributed ones.

copterrichie · Post by **copterrichie** » Fri Jun 08, 2012 9:34 pm

Being we have an open dialog going here, this is one reason I personally have resisted committing any code because I did not want to over write someone's hard work with my minor changes.

Repeat, we need a code review process.

Tommie · Post by **Tommie** » Fri Jun 08, 2012 9:38 pm

Actually, we need a distributed version control system. Reviewing is nice, but it does not help when the errors creep in during the merge process. At the moment, there is only one way to share code: commit it to the main repository. This sucks. Using a DVCS, everyone can has his own repository, merge code from any other person, review it, test it, decide to integrate it or whatever. It is far easier to reintegrate upstream changes. I _am_ using such a system at the moment, and I would not know how to handle development with multiple persons otherwise.

EOSBandi · Post by **EOSBandi** » Fri Jun 08, 2012 9:52 pm

I doubt that we will move to github. Even the SVN is sometime over the head of some contributors.

kos · Post by **kos** » Fri Jun 08, 2012 9:54 pm

just to make sure that this does not get over the head of anyone ;

http://svnbook.red-bean.com/en/1.7/svn. ... hatis.html

----------------------------------------------------
solving the issue with

SVN http://svnbook.red-bean.com/en/1.7/svn. ... rging.html

or

GIT http://progit.redhome.cc/book/ch6-7.html

there are some nice graphical tools fro both

code review is also a good practice

Tommie · Post by **Tommie** » Fri Jun 08, 2012 10:00 pm

There is no need to move to github. Github is just _one_ service hosting git repositories. Just use git/mercurial/bazaar, even Google Code suppports git.

It's far easier to develop new features in his own branch, rebase it from time to time and be actually able to check _yourself_ whether it integrates cleanly into upstream. SVNs branching and merging capabilities are a sick joke, and although I used it for many years, I sometimes wonder how I found that acceptable.

And don't underestimate the offline capabilities of git; svn cannot do anything without network connectivity. And if your repository servers is unreachable, manipulated or gone, you are fucked; your entire project history is gone. With git, everyone has a complete mirror of the entire history, protected by strong hash algorithms, and can work with that history _fast_.

Alexinparis · Post by **Alexinparis** » Fri Jun 08, 2012 10:08 pm

@Tommie,
Do you remember what we said in pm:

About the change to the code:
I think there is no problem to modify the code as long as it concerns a feature which is well isolated by #define. (like LED_FLASHER or LCD). The risk is a superposition of individual features non coordinated leading to a sort of patchwork, but it would take too much time for me to coordinate everything. So as long as your code is clean and you take time to document and communicate on it, it's ok

For structural changes like PWM generation, I2C code, RX code, AUX button code, ...
We need in all cases to discuss about it and have a whole agreement in the forum before changing something that already works.

Maybe there was some misandertand about this, but it concerns the shared_ part.
The shared_ part is not an open draft

Serial comm is a structural change.

The good way to communicate on a structural change is to write a prototype, communicate on it and convince others it's a good thing.

This is the way "code review" works here.
Generally speaking, there should be a general consensus to change things.
I'm fully confident with for instance Hamburger code about LCD, ronco code about PWM or EOSBandi about GPS, so one can feel _shared is fully open.

Saying that, some comments on the last changes:

- I think you are a brillant coder
- size of payload: notice that INBUF_SIZE if 64 octets. It is coherent with the old payload limitation.
- I like the serial protocol mod. one message type with lenght indication is probably better than 2.
- I like the way you code BOX.
- I think it could be extended also for PID
- I think it's still possible this way to build a GUI with grey parts for unused functions, it's just a matter of parsing checkbox strings and representation.
- I think you shouldn't touch what already works thank to kos for instance regarding the Serial transmission because currently the GUI is very laggy (1 update per second). So please, update in _shared only things you are sure of, and don't say that was not the good way to do (String allusion). If a thing can be better, no problem to change it, but it must work !
Individual char g_serial.write(b&0xFF); seems to be the problem in the GUI part.
edit: it's better in r873, but it's still more laggy than before and something seems to be still not ok.

- I'm agree with EOSBandi: GPS_distanceToHold and GPS_directionToHold should not be used. I see see the reason to include those variable over the Serial protocol. edit: removed since
- the i2c_init_done confusion is my fault because I forgot to include it in the sensor part when I merged the EOSBandi r33 GPS code.
- beware before changing type of variable from int16_t to uint16_t. side effect on computation should be studied carrefully. exemple: GPS_WP_RADIUS, waypoint_speed_gov and max_speed. I hope you checked carrefully the results with a flying machine proof.

Tommie · Post by **Tommie** » Fri Jun 08, 2012 10:26 pm

Alexinparis wrote:Serial comm is a structural change.

Sure, I got carried away; initially, I only intended to add a new message type (BOXNAMES), but after encountering limitation after limitation in the existing code, the stuff unraveled.

The good way to communicate on a structural change is to write a prototype, communicate on it and convince others it's a good thing.

- I think you are a brillant coder
- size of payload: notice that INBUF_SIZE if 64 octets. It is coherent with the old payload limitation.

Is there actually a message using that much payload data? I had to drastically reduce the serial buffers after the copter plowed into my furniture after a buffer overflow robbed me of complete control.
Using 1/4kB just for serial buffering on a device only employing 2kB doesn't seem like a good idea, so I am trying to cut corners on every static buffer I can find.

- I think you shouldn't touch what already works thank to kos for instance regarding the Serial transmission because currently the GUI is very laggy (1 update per second). So please, update in _shared only things you are sure of, and don't say that was not the good way to do (String allusion). If a thing can be better, no problem to change it, but it must work !

That's the catch. I once threw String handling out of the GUI when I noticed I could not calibrate my MAG or retrieve Debug values - funny enough, no one noticed that issue before. Then Strings got reintroduced, and when I had trouble with the GUI again, it was easy for me to find the culprit. It might be a local(e) issue here, but it didn't work, while it does now; sounds good to me.

edit: it's better in r873, but it's still more laggy than before and something seems to be still not ok.

The aggregation of the List<Byte> sequence can probably extended further.

- beware before changing type of variable from int16_t to uint16_t. side effect on computation should be studied carrefully. exemple: GPS_WP_RADIUS, waypoint_speed_gov and max_speed.

Well, signedness ws a problem before the change (compiler warning); that's the problem with the arduino IDE, one does not see the tremendous amount of warnings generated while compiling the code; therefore, possible bugs aren't noticed.

kos · Post by **kos** » Fri Jun 08, 2012 10:36 pm

so , tommie's changes remain in the _shared .. .?

@tommie : let us know when you consider it stable

Tommie · Post by **Tommie** » Fri Jun 08, 2012 10:58 pm

Hm, I'm using List<Byte> now to aggregate multiple requests; now it is too fast, the copter can't keep up and overwrites his answers in the TX buffer. I'll add some queuing machanism to prevent flooding the copter.

kos · Post by **kos** » Fri Jun 08, 2012 11:03 pm

erf .. try @ 57600

maybe we should rollback and create a branche for that ?

next release is not so far and shared must be maintained stable , agreement ?

Hamburger · Post by **Hamburger** » Sat Jun 09, 2012 1:33 pm

Tommie wrote:It was a working set of code. [..]. It was a bug, something that happens during development.

You submitted a set of code that failed badly on basic functionality.
That was your fault. You definitely did not test it.

You may not know this, but _shared is meant as a common basis for working code as in work in progress. It is not your personal playgorund. It is not ok to leave your basic coding errors for others to stumble upon.

If I was your parent I'd say 'an apology is in order'. Having followed your arrogance on others' work and your ignorance disguised as friendly 'lets discuss' while ignoring arguments presented leads me to question your qualities as a contributor to an ongoing open group effort.

I do not know how to work on something at the same time as you do, I will from now on avoid it.

Hamburger · Post by **Hamburger** » Sat Jun 09, 2012 1:34 pm

copterrichie wrote:Guys, seems to me, we need to implement the CODE review procedure before code is committed to the main or shared libraries. Google Code does have that feature.

Alex has made a concise statement on our 'code review'. Fine with me.

Hamburger · Post by **Hamburger** » Sat Jun 09, 2012 1:37 pm

kos wrote:so , tommie's changes remain in the _shared .. .?

@tommie : let us know when you consider it stable

+1
let us know when you consider it working again. Give it/us a break.

Hamburger · Post by **Hamburger** » Sat Jun 09, 2012 1:40 pm

kos wrote:next release is not so far and shared must be maintained stable , agreement ?

definitely.

Tommie · Post by **Tommie** » Sat Jun 09, 2012 4:03 pm

Hamburger wrote:You may not know this, but _shared is meant as a common basis for working code as in work in progress. It is not your personal playgorund. It is not ok to leave your basic coding errors for others to stumble upon.

Right, because code from _shared has always been completely reliable and never ever failed before. Like the time MAG calibration and debug data (along others) in the GUI did not work _for_weeks_. No one noticed, except me while tinkering with the sonar device, so I submitted a patch.
Or by the other time, when merging smashed two functions in the GPS code into each other, yielding a compiler error everytime GPS was included?
Or by the time some of the GPS changes made by EOSBandi were garbled by merging process, which only got noticed after I refactored some of it yesterday to remove compiler warnings?

I am sorry that the GUI lagged behind the copter for the tremendous time of roughly 2 hours, but please do not claim that the code in _shared was, is or ever will be that of sacrosanct clarity and perfection.

If I was your parent I'd say 'an apology is in order'. Having followed your arrogance on others' work and your ignorance disguised as friendly 'lets discuss' while ignoring arguments presented leads me to question your qualities as a contributor to an ongoing open group effort.

"I do not like it" is not a technical argument. It's the kind of subjective statement I like to ignore.

kos · Post by **kos** » Sat Jun 09, 2012 4:09 pm

like Alex suggested : lets extends the new msp to the pid settings , once this is done we should be ok .

fiendie · Post by **fiendie** » Sat Jun 09, 2012 4:23 pm

Hamburger wrote:You submitted a set of code that failed badly on basic functionality.
That was your fault. You definitely did not test it.

You may not know this, but _shared is meant as a common basis for working code as in work in progress. It is not your personal playgorund. It is not ok to leave your basic coding errors for others to stumble upon.

If I was your parent I'd say 'an apology is in order'. Having followed your arrogance on others' work and your ignorance disguised as friendly 'lets discuss' while ignoring arguments presented leads me to question your qualities as a contributor to an ongoing open group effort.

I do not know how to work on something at the same time as you do, I will from now on avoid it.

I think you've gone completely off the reservation.
Your accusations against Tommie are borderline paranoid and you should be careful not to poison the atmosphere just because things aren't handled your way.
You may not know this, but bugs happen all the time, with all the testing in the world. Last time I checked the changes were made to a Dev version. If there is no way to facilitate non-linear development problems like are likely to occur from time to time.

But your choice of words clearly shows what you're really up to here and it's got nothing to do with code quality.

Tommie · Post by **Tommie** » Sat Jun 09, 2012 4:29 pm

kos wrote:like Alex suggested : lets extends the new msp to the pid settings , once this is done we should be ok .

Done:
https://code.google.com/p/multiwii/source/detail?r=882
And the GUI has been updated as well, although it does not generate GUI elements from this message yet:
https://code.google.com/p/multiwii/source/detail?r=883

kos · Post by **kos** » Sat Jun 09, 2012 4:33 pm

Tommie wrote:
kos wrote:like Alex suggested : lets extends the new msp to the pid settings , once this is done we should be ok .

Done:
https://code.google.com/p/multiwii/source/detail?r=882
And the GUI has been updated as well, although it does not generate GUI elements from this message yet:
https://code.google.com/p/multiwii/source/detail?r=883

Ok ! I will extends the gui

and for any other changes that break compatibilities, create a svn branch first

Tommie · Post by **Tommie** » Sat Jun 09, 2012 4:57 pm

kos wrote:
Tommie wrote:
kos wrote:like Alex suggested : lets extends the new msp to the pid settings , once this is done we should be ok .

Done:
https://code.google.com/p/multiwii/source/detail?r=882
And the GUI has been updated as well, although it does not generate GUI elements from this message yet:
https://code.google.com/p/multiwii/source/detail?r=883

Ok ! I will extends the gui

I already have a prototype running, but it's a litle bit more complicated then the boxnames; we do have to store the scale data along with the name, as well as the data which controls are hidden from the user (no D or I).

copterrichie · Post by **copterrichie** » Sat Jun 09, 2012 5:03 pm

fiendie wrote:I think you've gone completely off the reservation.
Your accusations against Tommie are borderline paranoid and you should be careful not to poison the atmosphere just because things aren't handled your way.
You may not know this, but bugs happen all the time, with all the testing in the world. Last time I checked the changes were made to a Dev version. If there is no way to facilitate non-linear development problems like are likely to occur from time to time.

But your choice of words clearly shows what you're really up to here and it's got nothing to do with code quality.

You are correct about no amount of testing will assure bug free code however, if there is a review process before hand, there is a reduced chance of these bugs slipping in or by. With all RESPECTS to every member of this development team, the quality control/assurance could use a boost or a shot in the arm. I say this based upon my personal experience in Software development for major corporations.

Pyrofer · Post by **Pyrofer** » Sat Jun 09, 2012 5:23 pm

This is crazy.
There is a STABLE RELEASE. If you want stable working code, use that!
Then there is a place to do development 'shared', and that seems to be released in the dev branch.

Are you seriously saying there should never be a problem with the code ever? Where are you meant to put it so other people CAN try it?

"but _shared is meant as a common basis for working code as in work in progress."

WORK IN PROGRESS.

If people want rock solid code get it from stable. If people want work in progress that may not be perfect, it comes from dev, if you want the bleeding edge in development code its in shared.

If more than one person is going to contribute there needs to be a shared code base so people work from the same page. Hence shared! "Personal playground"??
If the changes had trashed work, set back the project or made things worse I could understand your problem but they didn't. As far as I can see your main issue appears to be that it was broken at the exact minute you checked out the code and you are looking for somebody to blame.

I've seen the code contributed and I can't wait to use it. We do not need to stifle development by picking on the people who are contributing greatly to the project.

kos · Post by **kos** » Sat Jun 09, 2012 5:33 pm

we are not refraining anyone .. there are some very basic rules

there is the stable branch : Alex ones

there is the "trunk" : shared

BOTH ARE STABLE

... no one should break the trunk or introduce major changes without agreement ,

what about osd and third party software relying on the serial protocol ?

so please stop flames and troll

Pyrofer wrote: if you want the bleeding edge in development code its in shared.

no, shared is not bleeding edge work in progress .. it is stable with community (aka shared) features .

Pyrofer · Post by **Pyrofer** » Sat Jun 09, 2012 5:47 pm

Funny, because dev 606 broke my quad.. It just doesn't work.
I reverted back to 504 and its fine.

I didn't bother to look for the bugs or the reason, but they are there. Good job on the quality control

Call me a troll, accuse me of flaming? Wow. All I want is progress and new features.
Not people moaning about work done because it was broken for a few minutes.

Go find the problem with dev 606 before complaining about shared being broken.

Alexinparis · Post by **Alexinparis** » Sat Jun 09, 2012 7:30 pm

Hi,

Some facts on code size based on r885 and the dev version 0606, and compiled with Arduino IDE (and no special gcc optimization) for a promini board:

1) just uncheck #define QUADX
based on r885:
code size: 13462
based on 0606:
code size: 11470

2) then uncheck BOARD_PROTO_1
based on r885:
code size: 23500
cycle time: mostly around 3200
the GUI is laggy (you can see it when ROLL or PITCH is moving on the right)

based on 0606:
code size: 22702
cycle time: mostly around 2800
the GUI is very smooth

3) then uncheck SERIAL_GPS
based on r885:
code size impossible: 31420
based on 0606:
code size: 30550

So for me, things are not going in the right direction.

Flash size is precious, probably more than ram size.
Serial consumes a lot of ram memory, but
“256 is choosen to avoid modulo operations on 8 bits pointers” was introduced for a good reason.
=> at least 0.5k burned in the recent changes. And reducing operations made inside an ISR is critical.
Serialize32 and serialize16 are no more equivalent in the current code
=> some bytes burned
I suppose FLAG mods consumes also more flash memory.

Before putting efforts on RAM size, I would like to see if there is currently a concern about the limit and where the octets are spent.

But the most important concerns:
- nearly 0.4ms cycle time more, what can explain this ??
- the GUI is laggy and it was not

So to sum up my view, things I like in the recent changes are:
- serial protocol mod
- PID and BOX name handled in the sketch

According to a recent post from Tommie: serial protocol mod + BOX mod should produce a code smaller than before. I hopped that.

I think it's important to converge on the serial protocol now, in order to release soon the 2.1 which is expected mainly for GPS and level mod improvement.

Tommie · Post by **Tommie** » Sat Jun 09, 2012 7:36 pm

Alexinparis wrote:Flash size is precious, probably more than ram size.
Serial consumes a lot of ram memory, but
“256 is choosen to avoid modulo operations on 8 bits pointers” was introduced for a good reason.

=> at least 0.5k burned in the recent changes. And reducing operations made inside an ISR is critical.
Serialize32 and serialize16 are no more equivalent in the current code
=> some bytes burned
I suppose FLAG mods consumes also more flash memory.

Before putting efforts on RAM size, I would like to see if there is currently a concern about the limit and where the octets are spent.

OK, so Flash size is more important than RAM?
What happens if you exceed flash? You notice while compiling, you might need a bigger controller, possibly spend some time getting it - although that is clearly not the case; things are working fine for my ATMega328p.
What happens if you exceed RAM? You notice when losing control, you might need a new copter, new furniture, possibly spend some time in the hospital. YES, STACK/HEAP OVERFLOWS DO have these consequences. Better have you insurance card at hand while flying your copter with all those precious saved flash memory.

(Yes, I experienced such overflows. It's not fun jumping after your copter before it plows into something you'd like to keep intact, like TV sets, mirrors, friends)

kos · Post by **kos** » Sat Jun 09, 2012 9:08 pm

Alexinparis wrote:- the GUI is laggy and it was not

fixed in shared

thanks to java : we can use the synchronized lag inside org.gnu.serial and the underlying jni to give the arduino enough time to empty his buffer

---------
about string stunts .. if users report some error then we may turn back and use the slower impl I provided here :

Code: Select all

// send msp without payload 
private List<Byte> requestMSP(int msp) {
       return  requestMSP( msp, null);
}

//send multiple msp without payload 
private List<Byte> requestMSP (int[] msps) {
  List<Byte> s = new LinkedList<Byte>(); 
  for (int m : msps) {
    s.addAll(requestMSP(m, null));
  }
  return s;
}

//send msp with payload 
private List<Byte> requestMSP (int msp, Character[] payload) {
  if(msp < 0) {
    return null; 
  }
  List<Byte> bf = new LinkedList<Byte>();
  for (byte c : MSP_HEADER.getBytes()) {
    bf.add( c );
  }

  byte checksum=0;
  byte pl_size = (byte)((payload != null ? int(payload.length) : 0)&0xFF);
  bf.add(pl_size);
  checksum ^= (pl_size&0xFF);

  bf.add((byte)(msp & 0xFF));
  checksum ^= (msp&0xFF);

  if (payload != null) {
    for (char c :payload){
      bf.add((byte)(c&0xFF));
      checksum ^= (c&0xFF);
    }
  }

  bf.add(checksum);
  return (bf);
}

void sendRequestMSP(List<Byte> msp) {
  byte[] arr = new byte[msp.size()];
  int i = 0;
  for (byte b: msp) {
    arr[i++] = b;
  }
  /* send the complete byte sequence in one go */
  g_serial.write(arr);
}

Alexinparis · Post by **Alexinparis** » Sat Jun 09, 2012 10:39 pm

Thank you kos, it works very well now.

some other checks on my side:
I think the increased cycle time is mainly due to FLAG mod via multiple calls to set_flag() and get_flag(), and its modulo, << and / operations

About free RAM,
http://arduino.cc/playground/Code/AvailableMemory

This simple function launched at different place shows we are far from seeing a ram memory run out
That there is at least 400 bytes free in all cases I tested with 256 octets serial buffer for TX and RX.

The critical size seems to be something like ~20 bytes left.
No function allocates 400 bytes locally.

int availableMemory() {
int size = 1024; // Use 2048 with ATmega328
byte *buf;
while ((buf = (byte *) malloc(--size)) == NULL)
;
free(buf);
return size;
}

PatrikE · Post by **PatrikE** » Sat Jun 09, 2012 11:03 pm

Shared serial dont work at all for me!.

Processing shows that checksuns is incorrect.
Type-- Expected 200 got 5....

Only gyrodata shows and updates in 2 sec intervall.

kos · Post by **kos** » Sat Jun 09, 2012 11:06 pm

does the led blink with the same 2sec refresh rate ?

edit ; tried re-uploading , reset , etc .. ?

PatrikE · Post by **PatrikE** » Sat Jun 09, 2012 11:19 pm

Nope the led blinks continuisly..
And Processing Floods error messages.

Tryed all that...

kos · Post by **kos** » Sat Jun 09, 2012 11:27 pm

PatrikE wrote:Nope the led blinks continuisly..
And Processing Floods error messages.

Tryed all that...

ok, you can test with the latest ?

http://code.google.com/p/multiwii/source/detail?r=891

and what is your board ?

Tommie · Post by **Tommie** » Sat Jun 09, 2012 11:36 pm

Now serial is broken again.The ATMega firmware cannot keep up with the number and speed of request that arrive at once; so the TX buffer fills and overwrites itself. This is why the checksums fail.

Code: Select all

invalid checksum for command 101: 21 expected, got 1
invalid checksum for command 36: 81 expected, got 1
invalid checksum for command 101: 137 expected, got 1
invalid checksum for command 101: 137 expected, got 1
invalid checksum for command 102: 224 expected, got 172
invalid checksum for command 36: 81 expected, got 1
invalid checksum for command 102: 224 expected, got 172
invalid checksum for command 101: 43 expected, got 1
invalid checksum for command 102: 225 expected, got 172
invalid checksum for command 101: 36 expected, got 1
invalid checksum for command 101: 173 expected, got 1
invalid checksum for command 101: 137 expected, got 1
invalid checksum for command 116: 117 expected, got 0
invalid checksum for command 102: 238 expected, got 172
invalid checksum for command 101: 141 expected, got 1
invalid checksum for command 101: 221 expected, got 1
invalid checksum for command 101: 69 expected, got 1
invalid checksum for command 102: 224 expected, got 172
invalid checksum for command 102: 224 expected, got 172
invalid checksum for command 101: 108 expected, got 1
invalid checksum for command 116: 117 expected, got 0
invalid checksum for command 101: 87 expected, got 1
invalid checksum for command 101: 236 expected, got 1
invalid checksum for command 116: 117 expected, got 0
invalid checksum for command 116: 117 expected, got 0
invalid checksum for command 101: 56 expected, got 1
invalid checksum for command 102: 235 expected, got 172
invalid checksum for command 101: 25 expected, got 1

We have to keep the serial code out of the draw() method; these two things keep tripping over each other. Use a thread to send the requests, this way, sending will not block the GUI.

Tommie · Post by **Tommie** » Sat Jun 09, 2012 11:37 pm

kos wrote:ok, you can test with the latest ?

I just did :-(

Tommie · Post by **Tommie** » Sat Jun 09, 2012 11:46 pm

Alexinparis wrote:This simple function launched at different place shows we are far from seeing a ram memory run out
That there is at least 400 bytes free in all cases I tested with 256 octets serial buffer for TX and RX.

With both buffers at 256 byte, I am down to 185 bytes, but I only placed the test function in two locations. As you can see, free memory can ary greatly depending on your sensor and software selection, and of course from the location you place your probe. Also remember that this value already factors in various removed variables (saved 20 bytes from rcValue, removed 16 byte with the servo array, 14 byte by removing the flag variables...); if you add that up and also factor in some ISR being triggered the wrong moment, I can quite see the stack waltzing through my heap.

// I did the math by stepping through the diff, and before cleaning up, additional 134 byte were bound in static variables, so our margin to disaster was down to 50 byte.

sismeiro · Post by **sismeiro** » Sun Jun 10, 2012 12:21 am

Tommie wrote:(Yes, I experienced such overflows. It's not fun jumping after your copter before it plows into something you'd like to keep intact, like TV sets, mirrors, friends)

It's bad what happened to you but fortunately you didn't harm yourself or other people. This is a reminder to all of us about the dangerous of this hobby and we can't be careless regarding any aspect of it. When testing developing code we need to be extra extra careful to avoid accidents, from now on I will be even more careful.

copterrichie · Post by **copterrichie** » Sun Jun 10, 2012 12:26 am

sismeiro wrote:It's bad what happened to you but fortunately you didn't harm yourself or other people. This is a reminder to all of us about the dangerous of this hobby and we can't be careless regarding any aspect of it. When testing developing code we need to be extra extra careful to avoid accidents, from now on I will be even more careful.

In my opinion, this applies to stable code as well.

sismeiro · Post by **sismeiro** » Sun Jun 10, 2012 12:54 am

copterrichie wrote:In my opinion, this applies to stable code as well.

Of course it does, I was not minimizing the implied danger whatever the nature of the code. That's why the "extra extra" and the "even more" I wrote.

copterrichie · Post by **copterrichie** » Sun Jun 10, 2012 1:07 am

sismeiro wrote:
copterrichie wrote:In my opinion, this applies to stable code as well.

Of course it does, I was not minimizing the implied danger whatever the nature of the code. That's why the "extra extra" and the "even more" I wrote.

Reason I stated that was, it was not the WMC but just a few days ago, I read where this guy had been using the same FC with no changes on hundreds of flights. One day, things changed and he came very close to some major injuries. I am in complete agreement with you about extra care, it just seems the moment we drop our guards or take things for granted, ... happens.

Tommie · Post by **Tommie** » Sun Jun 10, 2012 1:13 am

OK, back to the topic at hand. I added a second thread that handles the serial transmission, leaving the UI thread alone. It also introduces a request interval so the copter does not get flooded with requests; for the moment, I limited the transmission rate to one request each 20ms; At 115200bps, transmitting 128 byte of response payload takes 8.9ms; we also have more than on cycle time in reserve to let the firmware reach the serialCom functions.

Although speed limited, the GUI seems responsive and is working pretty well.

copterrichie · Post by **copterrichie** » Sun Jun 10, 2012 1:16 am

Just my two cents here but communication via the USB has never been a problem so I really don't understand the issue here. What I feel is more important is communication to OSDs, Bluetooth Telemetry and other third party applications.