Add BlueROV and BlueBoat120 params for ArduPilot 4.7.0 - #34
Conversation
Williangalvani
changed the title
vshie
left a comment
vshie
left a comment
There was a problem hiding this comment.
INS and GPS pos parameters identical - but is this due to external compass / gps on BlueBoat, or should we have position of accel/gyro/internal compass for INS pos?
Default of 6 for FS_Action should help quite a lot of situations.
Should RNGND!_Max be ping echosounder max range of 100m?
Does Temp_Log going true do something new in 4.7?
ES-Alexander
left a comment
ES-Alexander
left a comment
There was a problem hiding this comment.
Approving on grounds of urgency that the firmware is already released, and some valid defaults are better than no defaults. More thorough validation and subsequent updates can wait for future PRs (unless you're inclined to remove the commented stuff just before merging).
Per internal discussion, it would be good if we can (soon after this PR):
- review the desired behaviour for
ARMING_SKIPCHK - duplicate the 4.7 params for 4.8, so we have a starting point for current dev firmware
| FILT1_TYPE 0 | ||
| FILT2_TYPE 0 | ||
| FILT3_TYPE 0 | ||
| FILT4_TYPE 0 | ||
| FILT5_TYPE 0 | ||
| FILT6_TYPE 0 | ||
| FILT7_TYPE 0 | ||
| FILT8_TYPE 0 |
There was a problem hiding this comment.
This seems slightly weird to specify as part of vehicle defaults, but 🤷♂️
There was a problem hiding this comment.
should we blacklist these?
| FRSKY_DNLINK1_ID 20 | ||
| FRSKY_DNLINK2_ID 7 | ||
| FRSKY_DNLINK_ID 27 | ||
| FRSKY_OPTIONS 0 | ||
| FRSKY_UPLINK_ID 13 |
There was a problem hiding this comment.
We don't have a default RC radio setup, do we?
There was a problem hiding this comment.
should we blacklist these?
There was a problem hiding this comment.
I'd be inclined to, yeah.
| LOIT_TYPE 0 | ||
| MANUAL_OPTIONS 0 | ||
| MANUAL_STR_EXPO 0 | ||
| MAV1_ADSB 0 |
There was a problem hiding this comment.
Should vehicle parameters include MAVLink stream rates? To me that seems like something that should be left to the user / GCS.
There was a problem hiding this comment.
blacklist? I think there's value in keeping these... I bet we forget to ask some mesages in blueos
There was a problem hiding this comment.
I don't think they're super harmful given the user can still change them if they want, it just feels weird to me that we're being prescriptive over their default values without explicit thought/reasoning going into each one.
That said, no particularly strong feelings on this one 🤷♂️
| RNGFND1_ADDR 0 | ||
| RNGFND1_FUNCTION 0 | ||
| RNGFND1_GNDCLR 0.1 | ||
| RNGFND1_MAX 7 |
There was a problem hiding this comment.
Agreed with Tony that this is problematic. It should either be the Ping Sonar's range, or left unspecified.
3f65eef to
2c44e55
Compare
patrickelectric
left a comment
patrickelectric
left a comment
There was a problem hiding this comment.
I forgot to say, but, can we remove the spaces from the filenames ? We don't have them in any further file for our projects.
the filenames are what we show in vehicle setup, I think removing the space would look weird |
|
merging as the new stable is now available |
| %include "../../hardware/boards/pixhawk1.params" | ||
| %include "../base.params" |
There was a problem hiding this comment.
Should Patrick's suggestion on flipping these for Navigator also apply to the Pixhawk (i.e. base.params being the first inclusion)?
4 participants
No description provided.