Missing ; after macro, other avr-isms

[timecop]

I'm looking at current -dev again and hitting some bugs from indent -kr

if (accMode == 1) STABLEPIN_ON else STABLEPIN_OFF;
plz change to
if (accMode == 1) STABLEPIN_ON; else STABLEPIN_OFF;

[timecop]

in imu.pde,

#define fp_is_neg(val) ((((byte*)&val)[3] & 0x80) != 0)
plz change to

#define fp_is_neg(val) ((((uint8_t*)&val)[3] & 0x80) != 0)

[timecop]

#if not defined(TRUSTED_ACCZ)
to
#if !defined(TRUSTED_ACCZ)

1st version is not ANSI C

[timecop]

static uint32_t currentTime = 0;
static uint16_t previousTime = 0;

how could this ever work?

[Hamburger]

good.
Keep 'em coming.

[Alexinparis]

static uint32_t currentTime = 0;
static uint16_t previousTime = 0;

how could this ever work?

I think it was a try to save some space
We don't need the 32 bit accuracy here, but the arduino micro() returns a 32 bit var.
We appreciate your code review.

[timecop]

Yeah, but you're assigning 32bit currentTime to 16bit previous time which would overflow after like 65ms . Or am I missing something?

[Alexinparis]

Yeah, but you're assigning 32bit currentTime to 16bit previous time which would overflow after like 65ms . Or am I missing something?

yes, but previousTime is used only to calculate the cycle time which is hopefully far less than 65ms

[timecop]

While I am here, what the heck units does the hacked up atan2 use?
I replaced it with a regular atan2 and I can't match the output at all.
Is the final " z *= (180.0f / M_PI * 10); " part what I would need to do with real atan2 output to make it fit?

[timecop]

Yeah, but you're assigning 32bit currentTime to 16bit previous time which would overflow after like 65ms . Or am I missing something?

yes, but previousTime is used only to calculate the cycle time which is hopefully far less than 65ms

really, ok. so you're just abusing the fact that cycletime is also 16bit, and when doing currenttime-previoustime it overflows and becomes 16bit. boy i'm glad I don't have to worry about that on a real platform. also went through and trashed all those if blah < 350) { do calculation 16bit way } else { do it 32 bit way } lol

[Alexinparis]

While I am here, what the heck units does the hacked up atan2 use?
I replaced it with a regular atan2 and I can't match the output at all.
Is the final " z *= (180.0f / M_PI * 10); " part what I would need to do with real atan2 output to make it fit?

z *= (180.0f / M_PI * 10); is not a part of the real atan2 function,
it's a shortcut that is introduced here to factorize some code (it is called 3 times).
It returns an angle where one unit = 0.1deg

[Alexinparis]

boy i'm glad I don't have to worry about that on a real platform. also went through and trashed all those if blah < 350) { do calculation 16bit way } else { do it 32 bit way } lol

It's a little bit more than 100 microseconds gained on cycle time.
everything is good to take on this small 8 bit proc

[timecop]

Found another cute bug in latest -dev

Output.pde,

writeAllMotors(1000);
delay(300);

Surely that should be (MINCOMMAND) in there, for people who run for example XAircraft ESCs with 200.1000us range.

[kos]

yeah .. a new bug since r168 ..

[timecop]

More fail:

if (spekChannel < SPEK_MAX_CHANNEL) spekChannelData[spekChannel] = (long(spekFrame[b - 1] & SPEK_CHAN_MASK) << 8) + spekFrame[b];

long(blah) casting, this is C++ style.
plz 2 fix.

[Danal]

More fail:

if (spekChannel < SPEK_MAX_CHANNEL) spekChannelData[spekChannel] = (long(spekFrame[b - 1] & SPEK_CHAN_MASK) << 8) + spekFrame[b];

long(blah) casting, this is C++ style.
plz 2 fix.

What would you prefer to see?

[timecop]

The C-style of (long)(bla bla).
instead of long(bla bla).

[Alexinparis]

The C-style of (long)(bla bla).
instead of long(bla bla).

I still prefer (uint32_t)(blabla)

[timecop]

well, thats what I meant.
I saw you fixed it in pre3, thanks. my stm32 port is now synced to 2.0pre3 as well

this was just a question of style, (typecast)(value) vs typecast(value) for C vs C++

[Danal]

Well, the whole thing is gone in the new spektrum code that leverages the new serial code. There is a rough equivalent, and I will make it C and uint style.

Alex, I will fit the whole new spektrum code into shared this weekend.

[timecop]

Did this spektrum merge ever happen?

In unrelated note:

line ~200-ish of Multiwii.pde
// PITCH & ROLL only dynamic PID adjustemnt, depending on throttle value

Please fix this using braces into whatever you actually mean this should do.
EXTREMELY ambigious and breaks indent -kr

edit:
acc_LPF stuff
#define ACC_VALUE accSmooth[axis]
code later on directly uses accSmooth[]
so I think the #define is no longer necessary and just use accSmooth[] instead of accADC[] after.