motor_ramp: set fmu in test mode and main cleanup #11249

DanielePettenuzzo · 2019-01-20T17:25:32Z

This application was broken because it created a conflict on the pwm outputs. Now before starting the test we put the outputs in TEST mode until the test is over. Moreover a cleanup of the main function was done. Now arguments are handled with the px4_getopt function and the usage function contains more information.

Before the test could be done only on pwm_output0, while now we can use the -d flag to choose output1.

Tested on a pixhawk 4 but without motors so I just need to do a test on a vehicle before it is ready to merge.

DanielePettenuzzo · 2019-01-20T17:25:47Z

@RomanBapst FYI

LorenzMeier

Nice! Good to see this being simplified. I still would love to have motor test support via commands (already enabled in Auterion GS) in the UI. But that doesn't impact this PR. Please merge after on-HW test.

dagar · 2019-01-21T01:12:07Z

Motor test would be extremely helpful for new setups. It's a frequent request.

Existing issue and related discussion.
#10782
#9544 (comment)

Safety concerns were one of the reasons I never got around to plumbing this together with the existing MAV_CMD_DO_MOTOR_TEST command.

DanielePettenuzzo · 2019-01-21T12:31:26Z

@LorenzMeier I tested it a bit and found out that the pwm min was not restored at the end of the test. I pushed a commit that fixes it. I think we can merge this.

DanielePettenuzzo · 2019-01-31T21:11:04Z

@LorenzMeier @bkueng Rebased. I think we can merge this

bkueng

Thanks for the cleanup!

bkueng · 2019-02-04T10:20:09Z

src/systemcmds/motor_ramp/motor_ramp.cpp

 static Mode _mode;
 static char *_mode_c;
+static char *_pwm_output_dev = "/dev/pwm_output0";


Suggested change

static char *_pwm_output_dev = "/dev/pwm_output0";

static const char *_pwm_output_dev = "/dev/pwm_output0";

bkueng · 2019-02-04T10:20:48Z

src/systemcmds/motor_ramp/motor_ramp.cpp

+			case 'r':
+				_ramp_time = atof(myoptarg);
+
+				if (!(_ramp_time > 0)) {


Suggested change

if (!(_ramp_time > 0)) {

if (_ramp_time <= 0) {

bkueng · 2019-02-04T10:22:32Z

src/systemcmds/motor_ramp/motor_ramp.cpp

 	}

-	if (!(_ramp_time > 0)) {
-		usage("ramp time must be greater than 0");
+	_mode_c = const_cast<char*>(myoptarg);


Please change _mode_c to const char*

Can you remove the cast?

bkueng · 2019-02-04T10:27:41Z

src/systemcmds/motor_ramp/motor_ramp.cpp

+static char *_pwm_output_dev = "/dev/pwm_output0";
+static struct pwm_output_values _last_spos;
+static struct pwm_output_values _last_min;
+static unsigned _servo_count;


Please move all of these 3 into motor_ramp_thread_main.

bkueng · 2019-02-04T10:28:55Z

src/systemcmds/motor_ramp/motor_ramp.cpp

 static int _min_pwm;
-static int _max_pwm;
+static int _max_pwm = 2000;


Please reset these default values inside motor_ramp_main, at the very top. Otherwise you get inconsistent results, e.g. with:

motor_ramp sine -a 1100 -r 0.5 -b 1800 motor_ramp sine <- will use 1800 as max pwm, not 2000.

src/systemcmds/motor_ramp/motor_ramp.cpp

DanielePettenuzzo · 2019-04-01T08:27:17Z

@bkueng rebased and added a commit fixing stuff based on your review

bkueng · 2019-04-01T12:05:29Z

src/systemcmds/motor_ramp/motor_ramp.cpp

-		_mode = SQUARE;
+			case 'd':
+				if(!strcmp(myoptarg, "/dev/pwm_output0") || !strcmp(myoptarg, "/dev/pwm_output1")){
+					_pwm_output_dev = const_cast<char*>(myoptarg);


The cast should not be required.

Suggested change

_pwm_output_dev = const_cast<char*>(myoptarg);

_pwm_output_dev = myoptarg;

bkueng · 2019-04-01T12:06:12Z

src/systemcmds/motor_ramp/motor_ramp.cpp

 	}

-	if (!(_ramp_time > 0)) {
-		usage("ramp time must be greater than 0");
+	_mode_c = const_cast<char*>(myoptarg);


Can you remove the cast?

src/systemcmds/motor_ramp/motor_ramp.cpp

… before returning

bresch · 2019-08-14T13:59:44Z

@bkueng Are we good now?

bkueng

Thanks for the update. Found some more things.

bkueng · 2019-08-14T14:47:49Z

src/systemcmds/motor_ramp/motor_ramp.cpp

+
+	if(!set_pwm_min){
+		PX4_WARN("pwm_min not set. use -a flag");
+		error_flag = 1;


Suggested change

error_flag = 1;

error_flag = true;

bkueng · 2019-08-14T14:50:18Z

src/systemcmds/motor_ramp/motor_ramp.cpp


 	if (fd < 0) {
-		PX4_ERR("can't open %s", dev);
+		PX4_ERR("can't open %s", _pwm_output_dev);


Missing a return 1
Also _thread_running needs to be set to false on each return.

bkueng · 2019-08-14T14:51:18Z

src/systemcmds/motor_ramp/motor_ramp.cpp

@@ -413,8 +494,28 @@ int motor_ramp_thread_main(int argc, char *argv[])
 	}

 	if (fd >= 0) {
-		/* disarm */
-		ioctl(fd, PWM_SERVO_DISARM, 0);
+		/* get current pwm min */


Suggested change

/* get current pwm min */

/* set current pwm min */

bkueng · 2019-08-14T14:51:33Z

src/systemcmds/motor_ramp/motor_ramp.cpp

-		ioctl(fd, PWM_SERVO_DISARM, 0);
+		/* get current pwm min */
+		if (px4_ioctl(fd, PWM_SERVO_SET_MIN_PWM, (long unsigned int)&last_min) < 0) {
+			PX4_ERR("failed getting pwm min values");


Suggested change

PX4_ERR("failed getting pwm min values");

PX4_ERR("failed setting pwm min values");

…aste typos

bresch · 2019-08-14T15:13:42Z

@bkueng Done

and restore pwm min after test

LorenzMeier previously approved these changes Jan 20, 2019

View reviewed changes

DanielePettenuzzo dismissed LorenzMeier’s stale review via 0d0aced January 21, 2019 12:27

DanielePettenuzzo changed the title ~~[WIP] motor_ramp: set fmu in test mode and main cleanup~~ motor_ramp: set fmu in test mode and main cleanup Jan 21, 2019

weekly-digest bot mentioned this pull request Jan 27, 2019

Weekly Digest (20 January, 2019 - 27 January, 2019) #11307

Closed

DanielePettenuzzo force-pushed the pr-motor-ramp-fixes branch from 0d0aced to add7f05 Compare January 31, 2019 21:10

bkueng reviewed Feb 4, 2019

View reviewed changes

weekly-digest bot mentioned this pull request Feb 10, 2019

Weekly Digest (3 February, 2019 - 10 February, 2019) #11425

Closed

DanielePettenuzzo force-pushed the pr-motor-ramp-fixes branch from add7f05 to 8aed261 Compare April 1, 2019 08:22

bkueng reviewed Apr 1, 2019

View reviewed changes

stale bot added the Admin: Wont fix label Jul 10, 2019

weekly-digest bot mentioned this pull request Jul 14, 2019

Weekly Digest (7 July, 2019 - 14 July, 2019) #12474

Closed

PX4 deleted a comment from stale bot Jul 15, 2019

stale bot removed the Admin: Wont fix label Jul 15, 2019

weekly-digest bot mentioned this pull request Jul 21, 2019

Weekly Digest (14 July, 2019 - 21 July, 2019) #12528

Closed

bresch added this to the Release v1.10.0 milestone Aug 14, 2019

bresch added the bug label Aug 14, 2019

DanielePettenuzzo added 3 commits August 14, 2019 10:44

motor_ramp: set fmu in test mode and main cleanup

6a24669

motor ramp: restore pwm min after test

53917d4

minor fixes for motor_ramp pr

4e28128

bresch force-pushed the pr-motor-ramp-fixes branch from 8aed261 to 9dccb5a Compare August 14, 2019 13:47

Motor ramp - remove const_cast where not needed and close fd if error…

c7bf569

… before returning

bresch force-pushed the pr-motor-ramp-fixes branch from 9dccb5a to c7bf569 Compare August 14, 2019 13:59

bkueng reviewed Aug 14, 2019

View reviewed changes

Motor ramp - Set thread_running to false before returning, fix copy-p…

5c6cf86

…aste typos

Merge branch 'master' into pr-motor-ramp-fixes

c5631b5

bkueng approved these changes Aug 15, 2019

View reviewed changes

bkueng merged commit 82ecf4d into master Aug 15, 2019

bkueng deleted the pr-motor-ramp-fixes branch August 15, 2019 07:42

weekly-digest bot mentioned this pull request Aug 18, 2019

Weekly Digest (11 August, 2019 - 18 August, 2019) #12733

Closed

bozkurthan pushed a commit to bozkurthan/Firmware that referenced this pull request Sep 4, 2019

motor_ramp: set fmu in test mode and cleanup (PX4#11249)

b81ae74

and restore pwm min after test

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

motor_ramp: set fmu in test mode and main cleanup #11249

motor_ramp: set fmu in test mode and main cleanup #11249

DanielePettenuzzo commented Jan 20, 2019 •

edited by AuterionWrikeBot

Loading

DanielePettenuzzo commented Jan 20, 2019

LorenzMeier left a comment •

edited

Loading

dagar commented Jan 21, 2019 •

edited

Loading

DanielePettenuzzo commented Jan 21, 2019

DanielePettenuzzo commented Jan 31, 2019

bkueng left a comment

bkueng Feb 4, 2019

bkueng Feb 4, 2019

bkueng Feb 4, 2019

bkueng Apr 1, 2019

bkueng Feb 4, 2019

bkueng Feb 4, 2019

DanielePettenuzzo commented Apr 1, 2019

bkueng Apr 1, 2019

bkueng Apr 1, 2019

bresch commented Aug 14, 2019

bkueng left a comment

bkueng Aug 14, 2019

bkueng Aug 14, 2019

bkueng Aug 14, 2019

bkueng Aug 14, 2019

bresch commented Aug 14, 2019

	static char *_pwm_output_dev = "/dev/pwm_output0";
	static const char *_pwm_output_dev = "/dev/pwm_output0";

	_pwm_output_dev = const_cast<char*>(myoptarg);
	_pwm_output_dev = myoptarg;

	PX4_ERR("failed getting pwm min values");
	PX4_ERR("failed setting pwm min values");

motor_ramp: set fmu in test mode and main cleanup #11249

motor_ramp: set fmu in test mode and main cleanup #11249

Conversation

DanielePettenuzzo commented Jan 20, 2019 • edited by AuterionWrikeBot Loading

DanielePettenuzzo commented Jan 20, 2019

LorenzMeier left a comment • edited Loading

Choose a reason for hiding this comment

dagar commented Jan 21, 2019 • edited Loading

DanielePettenuzzo commented Jan 21, 2019

DanielePettenuzzo commented Jan 31, 2019

bkueng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielePettenuzzo commented Apr 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bresch commented Aug 14, 2019

bkueng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bresch commented Aug 14, 2019

DanielePettenuzzo commented Jan 20, 2019 •

edited by AuterionWrikeBot

Loading

LorenzMeier left a comment •

edited

Loading

dagar commented Jan 21, 2019 •

edited

Loading