Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rcout script changes #12527

Merged
merged 3 commits into from
Jul 31, 2019
Merged

rcout script changes #12527

merged 3 commits into from
Jul 31, 2019

Conversation

garfieldG
Copy link
Contributor

@garfieldG garfieldG commented Jul 21, 2019

-Fixed bug : cd to /etc/init.d/airframes, but no cd after
-Added error report to user if value of SYS_AUTOSTART param value is incorrect

Copy link
Contributor

@julianoes julianoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

Tools/px4airframes/rcout.py Outdated Show resolved Hide resolved
@@ -50,14 +50,23 @@ def __init__(self, groups, board):
result += "# %s\n" % param.GetName()
result += "if param compare SYS_AUTOSTART %s\n" % id_val
result += "then\n"
result += "\tsh %s\n" % path
result += "\tsh /etc/init.d/airframes/%s\n" % path
result += "\tset SYS_AUTOSTART_VALUE %s\n" % id_val
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This increases flash usage (that is why there was a cd).
Can you find a solution that does not add to to each airframe?
For example:

result +=   "\set F %s\n" % path

And then you can check if F is valid at the end.

Also we'll need to coordinate with #12526.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkueng
Thanks for the feedback!
I changed it so to reduce the flash usage from my previous commit.
also, coordinated it with #12526

-Fixed bug : cd to /etc/init.d/airframes, but no cd after
-Added error report to user if value of SYS_AUTOSTART param value is incorrect
-reduced flash usage
-now supports airframe .post scripts
@@ -62,14 +63,28 @@ def __init__(self, groups, board, post_start=False):
result += "# %s\n" % param.GetName()
result += "if param compare SYS_AUTOSTART %s\n" % id_val
result += "then\n"
result += "\tsh %s\n" % path
if post_start:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you keep the logic the same for post_start? The only thing that needs to be different is not to print an error if there is no .post file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkueng
you want to keep the logic with AIRFRAME for .post files?
I did it that way so that if there's no .post file rc.autostart.post will be empty, if we'll continue the same logic the script will never be empty :
set AIRFRAME none
if [ ${AIRFRAME} != none ]
sh /etc/init.d/airframes/${AIRFRAME}
fi
unset AIRFRAME

I tried to avoid it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a small optimization, so that reasoning works too. I slightly prefer to be consistent here, but either way is fine with me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @bkueng, it is better to have the same logic and same output format for both files.
Flash-wise, the 5 lines you show should be OK.

Copy link
Contributor Author

@garfieldG garfieldG Jul 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bkueng @jlecoeur
changed it to be same logic.

Same logic for non .post files and .post files
Copy link
Contributor

@jlecoeur jlecoeur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

I tested on fmu-v5 that a .post file is correctly run, and that a boot error is issued when an incorrect autostart ID is entered.

@bkueng bkueng merged commit 487addb into PX4:master Jul 31, 2019
@garfieldG garfieldG deleted the rcout branch July 31, 2019 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants