General servo handler - almost done

This forum is dedicated to software development related to MultiWii.
It is not the right place to submit a setup problem.
Software download
timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: General servo handler - almost done

Post by timecop »

Mis wrote:This is high level of "C" coding :lol: : value = condition ? value_if_true : value_if_false; This is easy :D
If sevo[nr].middle have value less than RC_CHANS (e.g 0..11) this function return value from rc channel table indexed by servo[nr].middle. Example: If servo.middle have value of 5, then this function return value from rcData[5] that is eqievalent of AUX2 value. This is a part of conditional "(conf.servoConf[nr].middle < RC_CHANS)" statement before ":" if the condition is true.
In other case, if servo.middle is bigger than 12 (e.g 1500 or 1700) this function return directly the servo.middle value (1500 or 1700 in this case). This is a part of conditional "(conf.servoConf[nr].middle < RC_CHANS)" statement after ":" if the condition is false.
With this way you can assign predefined value for servo middle position, or assign any RC Channel for control servo middle position (setting value 0..11).


I know what C is.
I also know that servo[].middle is commented in the header to be "SERVO PWM VALUE, DEFAULT OF 1500".
So why the fuck it would EVER be an index into rcData[] array is absolutely fucking mindboggling.
This is not "high level of coding", this is high level of obfuscation and idiocy.
I"m not complaining about the ternary operator, I'm well aware of how it works.
I'm complaining about reusing a variable that's supposed to be a servo middle PWM value as an index into something else without absolutely any commenting or explanation or reason for why this is done.

Code: Select all

struct servo_conf_ {            // this is a generic way to configure a servo, every multi type with a servo should use it
    int16_t min;                // minimum value, must be more than 1020 with the current implementation
    int16_t max;                // maximum value, must be less than 2000 with the current implementation
    int16_t middle;             [b]// default should be 1500[/b]
    int8_t rate;                // range [-100;+100] ; can be used to ajust a rate 0-100% and a direction
};

User avatar
ezio
Posts: 827
Joined: Sun Apr 01, 2012 11:03 pm
Location: Paris
Contact:

Re: General servo handler - almost done

Post by ezio »

timecop wrote:
Mis wrote:This is high level of "C" coding :lol: : value = condition ? value_if_true : value_if_false; This is easy :D
If sevo[nr].middle have value less than RC_CHANS (e.g 0..11) this function return value from rc channel table indexed by servo[nr].middle. Example: If servo.middle have value of 5, then this function return value from rcData[5] that is eqievalent of AUX2 value. This is a part of conditional "(conf.servoConf[nr].middle < RC_CHANS)" statement before ":" if the condition is true.
In other case, if servo.middle is bigger than 12 (e.g 1500 or 1700) this function return directly the servo.middle value (1500 or 1700 in this case). This is a part of conditional "(conf.servoConf[nr].middle < RC_CHANS)" statement after ":" if the condition is false.
With this way you can assign predefined value for servo middle position, or assign any RC Channel for control servo middle position (setting value 0..11).


I know what C is.
I also know that servo[].middle is commented in the header to be "SERVO PWM VALUE, DEFAULT OF 1500".
So why the fuck it would EVER be an index into rcData[] array is absolutely fucking mindboggling.
This is not "high level of coding", this is high level of obfuscation and idiocy.
I"m not complaining about the ternary operator, I'm well aware of how it works.
I'm complaining about reusing a variable that's supposed to be a servo middle PWM value as an index into something else without absolutely any commenting or explanation or reason for why this is done.

Code: Select all

struct servo_conf_ {            // this is a generic way to configure a servo, every multi type with a servo should use it
    int16_t min;                // minimum value, must be more than 1020 with the current implementation
    int16_t max;                // maximum value, must be less than 2000 with the current implementation
    int16_t middle;             [b]// default should be 1500[/b]
    int8_t rate;                // range [-100;+100] ; can be used to ajust a rate 0-100% and a direction
};

+1

timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: General servo handler - almost done

Post by timecop »

OK, so, next:

Code: Select all

#define SERVODIR(n,b) ((conf.servoConf[n].rate & b) ? -1 : 1)


what? Rate is a bitfield now?
I see it used like this:
SERVODIR(5, 2) * axisPID[YAW]
so
if (servo[5].rate & 0x02)
return -1;
else
return 1;
so what the ?!?@?$@#??@#?$ is the valid range of values for rate??? & 2 is gonna be active at every number whose 2nd LSB bit is set... what???
Looking at processing source (ugh) it seems numbers > 127 = negative, < 127 = positive... That makes a bit more sense...

also:

Code: Select all

int8_t rate;                            // range [-100;+100] ; can be used to ajust a rate 0-100% and a direction

Mis
Posts: 203
Joined: Fri Apr 01, 2011 12:23 am

Re: General servo handler - almost done

Post by Mis »

The rate value have two different meanings.
With some cases (e.g airplane) the rate value is exactly the single servo rate with -128 .. +127 value. This define single servo amplify and servo direction.
In other cases (e.g flying wing) the rate value work only as reverse flags. Rate value can store up to 8 reverse flags coded as bitfields. In case flying wing we need two reverse flags for any of two aileron servos (one reverse from pitch, second reverse from roll axis). In this case flag at bit0 work as reverse for pitch axis, and bit1 work as reverse for roll axis. In this case the rate value should be only 0,1,2,3.

timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: General servo handler - almost done

Post by timecop »

I also found a third usecase... where rate[] is used for gimbal gain....
how about writing some comments in code? you know, they don't increase compile size on 328mega...

timecop
Posts: 1880
Joined: Fri Sep 02, 2011 4:48 pm

Re: General servo handler - almost done

Post by timecop »

How does SERVO_TILT now work with controlling pitch/roll from AUX3/4? (or any other channel)?
MID selector in mwii GUI doesn't go below 1200.
part2:
> if (i != 5) // not limit YawMotor
Why?
I don't see it being constrained anywhere else in the code.
> servo[i] = constrain(servo[i], conf.servoConf[i].min, conf.servoConf[i].max); // limit the values

PatrikE
Posts: 1976
Joined: Tue Apr 12, 2011 6:35 pm
Location: Sweden
Contact:

Re: General servo handler - almost done

Post by PatrikE »

timecop wrote:How does SERVO_TILT now work with controlling pitch/roll from AUX3/4? (or any other channel)?
MID selector in mwii GUI doesn't go below 1200.

Actually it does.
The slider have dual ranges.
Pull the slider all the way past 1200 and it will switch range to RC-channels.
Return to Mid setting by pulling it full to the right.

Mis
Posts: 203
Joined: Fri Apr 01, 2011 12:23 am

Re: General servo handler - almost done

Post by Mis »

part2:
> if (i != 5) // not limit YawMotor
Why?
I don't see it being constrained anywhere else in the code.


This is taken from old multiwii 2.1 helicopter code. The yawmotor can be set to mincommand during disarmed, but after constrain to servo.min (the lower value of servo.min is 1020) the yawmotor can't stop when disarmed. This can be ommited, but need extend min servo limits to 1000. But maybe current 1020 should be sufficient.

BTW I don't see possibility to set servo.min and servo.max with current GUI for planes. Only midpoint, rate and reverse are available for planes servos.
Limits should be available to avoid mechanical damage if the travel is too high for some planes construction.

dominicclifton
Posts: 202
Joined: Tue Feb 05, 2013 10:28 pm

Re: General servo handler - almost done

Post by dominicclifton »

timecop wrote:I also know that servo[].middle is commented in the header to be "SERVO PWM VALUE, DEFAULT OF 1500".
So why the fuck it would EVER be an index into rcData[] array is absolutely fucking mindboggling.
This is not "high level of coding", this is high level of obfuscation and idiocy.
I"m not complaining about the ternary operator, I'm well aware of how it works.
I'm complaining about reusing a variable that's supposed to be a servo middle PWM value as an index into something else without absolutely any commenting or explanation or reason for why this is done.


+1 MILLION

OMFG - DO NOT EVER RE-USE VARIABLES! BAD BAD BAD BAD BAD!

dominicclifton
Posts: 202
Joined: Tue Feb 05, 2013 10:28 pm

Re: General servo handler - almost done

Post by dominicclifton »

Putting my money where my mouth is, I've implemented a solution in my git repo for baseflight:

https://github.com/hydra/baseflight/com ... 18ed64b9aa

I think it should be back-ported into MultiWii too.

AkaMrBill
Posts: 31
Joined: Tue Jan 14, 2014 5:40 pm
Location: Elk Grove, Ca

Re: General servo handler - almost done

Post by AkaMrBill »

I know this is a bit dated, but this thread is the closest thing I have found that addresses what I am dealing with.

Because of this thread, I found out how to use the GUI to set up an AUX ch to control the center of the Tilt servo for my gimbal.
Well, at least in theory. See, it doesn't change anything. Even adjusting the travel limits and tilt_prop do nothing.
Yes, I have selected the "Go Live" button. Even if I change the values in the GUI and click "Write", if I then click "Read", the default values then repopulate the GUI.
I believe the problem is that the eeprom is not being accessed nor written to from the "Servo" tab.

I am running this on a Windows 8 64bit system, but I am using the 32 bit version of MultiWiiConf 2.3, although I am not quite certain of the build version.

PatrikE
Posts: 1976
Joined: Tue Apr 12, 2011 6:35 pm
Location: Sweden
Contact:

Re: General servo handler - almost done

Post by PatrikE »

Test to press write from the PID Tab.
Just to se if it makes any difference.

PatrikE
Posts: 1976
Joined: Tue Apr 12, 2011 6:35 pm
Location: Sweden
Contact:

Re: General servo handler - almost done

Post by PatrikE »

After some testing and only found situation settings isn't saved.
If LIVE button is RED when you adjust the settings.
The servo settings is only sent to controller when LIVE button is GREEN.

Save Button sends Save Eeprom command regardless.
But It saves only the current settings in the controller.( Not the visual in Gui if servos isn't LIVE)

AkaMrBill
Posts: 31
Joined: Tue Jan 14, 2014 5:40 pm
Location: Elk Grove, Ca

Re: General servo handler - almost done

Post by AkaMrBill »

PatrikE wrote:After some testing and only found situation settings isn't saved.
If LIVE button is RED when you adjust the settings.
The servo settings is only sent to controller when LIVE button is GREEN.

Save Button sends Save Eeprom command regardless.
But It saves only the current settings in the controller.( Not the visual in Gui if servos isn't LIVE)


Thanks for replying!
Yes, the Live button was green.
That's why this is so frustrating. I was trying to follow the proper usage, but it made no effect.
When "Live", adjusting the Prop slider would not change the response of the servo. Changing the middle would not either. Clicking on "Write" didn't seem to access the eeprom. I say that because if I clicked "READ" after "Write", it would re-load the default values for the servo. BTW, I see the same behavior when working with the tail servo for my Tri.
So, it clearly must be my PC, or my version of MultiWiiConf that is goofy.
Any suggestions on how to verify this, and correct it?

PatrikE
Posts: 1976
Joined: Tue Apr 12, 2011 6:35 pm
Location: Sweden
Contact:

Re: General servo handler - almost done

Post by PatrikE »

When the servos is "Live" the servo should move when you move the center slider.
When you press read the actual values in FC "RAM memory" will be received.
Pressing save/Write will burn settings to Eeprom.

If you change without pressing write or save the changes is lost in next reboot.

If it's possible adjust servos center from GUI then the settings is possible to save to Eeprom.

It sounds like your FC don't receive the settings from Gui.
Use V2.3 Both MWii & MWii config.

AkaMrBill
Posts: 31
Joined: Tue Jan 14, 2014 5:40 pm
Location: Elk Grove, Ca

Re: General servo handler - almost done

Post by AkaMrBill »

PatrikE wrote:...

If it's possible adjust servos center from GUI then the settings is possible to save to Eeprom.

It sounds like your FC don't receive the settings from Gui.
Use V2.3 Both MWii & MWii config.


Exactly!!
I am using both MWii and MWii config v2.3. So we are in complete agreement there.
I don't mind re-installing MWii Config to make certain I have the right version. Just to make certain I am using the latest and correct version, where should I download it from?
I doubt it, but have to ask, is there any chance running the 32bit version of MWii config on a 64bit OS could possibly be causing this?

******************************** EDIT ********************************************
I just tried clearing the eeprom, then flashing MW 2.3 again.
No change.

I then removed the USB drivers and re-installed them.
No change.

It still acts as though FC is not getting settings from the GUI.

*********************** EDIT #2 **********************************************

I just tried this same thing on a completely different computer with the same result.

Although, it too is running Windows 8.

Running in Windows 7 compatibility mode made no difference either.

*************************** EDIT #3 ********************************************
I just did a fresh install of the USB drivers and the MW GUI on a Windows XP laptop.

Still no effect when changing parameters on the SERVO tab.

Now I'm really starting to second guess myself!!
I have followed the directions for the GUI's use to the letter but to no effect.

The FC just doesn't see the settings from the GUI.

Post Reply