Seems ok to me, although I’d personally prefer a naming structure that’s generalisable to other fully vectored frames with different numbers of thrusters, as I suggested in the issue:
Maybe SUB_FRAME_OMNI8_6DOF, to maintain consistency but also keep some semblance of brevity?
Regardless, the name should be easy enough to change if need be, so if you make your PR @williangalvani can review it and we can get it merged in after any issues are resolved 
I have been thinking about this a little bit today and there are a few distinct flavours of FULLY_VECTORED8. I think there are 4 flavours, but only 3 make sense to use. Someone is going to need to sell me on why OMNI should be used.
One where the motors point inwards towards the dome like the SRV-8:
Another where they point away from the dome and towards the vertical plane like the boxfish:
And a third pointing away from the dome and towards the horizontal plane like Etienne’s model:
These configurations would need to be distinct as this would change the vector table.
Haha, it’s not a requirement, just might be a bit friendlier than FULLY_VECTORED when combined with the existing name components. Overly long names can become a bit unwieldy to use, but if you find it clearer the other way then you’re welcome to submit your PR the way you want to
Hmmm, not sure how that should best be handled. They’re fully defined by a single thruster (assuming similar geometry), but there are technically 8 different configurations (two of which likely aren’t useful), and naming 6 extra configurations that are very similar seems quite painful.
I’ve had this response half-written for multiple days now - I wrote some code a few days ago to generate the different configurations, which I think is correct (it seems to be generating the BlueROV2 ones correctly at least), and I was planning to add some basic plotting for confirmation but haven’t got around to it yet.
I discussed this a bit internally and @williangalvani mentioned we can likely follow ArduCopter’s approach of making the frame definition dynamically adjustable with lua scripting, in which case new frames wouldn’t even require building ArduSub or flashing a new firmware. That’s something I’ve wanted for a while, but it would still need some kind of templating built around it to be simple to use for common configurations (presumably it’s still annoying to have to specify all the values manually in a script, even if doing so means you get to avoid having to build and flash the firmware).
Hello Eliot,
Sorry it took a while for be to get back at this. We are almost ready for testing our new ROV
I followed the instructions on here: Build ArduSub · GitBook
Added my custom setup in AP_Motors6DOF.cpp
case SUB_FRAME_CUSTOM:
_frame_class_string = "DROV-8VEC";
add_motor_raw_6dof(AP_MOTORS_MOT_1, -1.0f, -1.0f, 1.0f, 1.0f, 1.0f, 1.0f, 1);
add_motor_raw_6dof(AP_MOTORS_MOT_2, 1.0f, -1.0f, -1.0f, 1.0f, 1.0f, -1.0f, 2);
add_motor_raw_6dof(AP_MOTORS_MOT_3, -1.0f, 1.0f, -1.0f, 1.0f, -1.0f, 1.0f, 3);
add_motor_raw_6dof(AP_MOTORS_MOT_4, 1.0f, 1.0f, 1.0f, 1.0f, -1.0f, -1.0f, 4);
add_motor_raw_6dof(AP_MOTORS_MOT_5, 1.0f, 1.0f, 1.0f, -1.0f, 1.0f, 1.0f, 5);
add_motor_raw_6dof(AP_MOTORS_MOT_6, -1.0f, 1.0f, -1.0f, -1.0f, 1.0f, -1.0f, 6);
add_motor_raw_6dof(AP_MOTORS_MOT_7, 1.0f, -1.0f, -1.0f, -1.0f, -1.0f, 1.0f, 7);
add_motor_raw_6dof(AP_MOTORS_MOT_8, -1.0f, -1.0f, 1.0f, -1.0f, -1.0f, -1.0f, 8);
break;
Loaded it through QGC firmware update.
I am getting this error with the new version.
The compass seems to be working fine.
Is this something you came across before?
Is there a way to see if my config is properly loaded before I test it?
Cheers,
E.
Yeah, that’s a known parameter mismatch (QGC 4.0 doesn’t have full support for ArduSub >= 4.1), but it should still work fine. That said, you can also update to QGC v4.1.6 (our latest recommended version) to fix the warning
You can check that FRAME_CONFIG is set to Custom, and ensure that the Motors Setup Page is showing all 8 of your motors - I believe when using a custom frame no frame image will appear. From there I’d suggest using the motor test sliders to confirm the motors are all connected to the correct locations, and then you can either set the relevant Reverse checkboxes while doing that, or use the “Auto-Detect Directions” button once the vehicle is in the water (which I believe should work, but may struggle given the less strong alignment of motors in this configuration with each vehicle rotation angle)
Hi Eliot,
Thanks for the info.
I was finally able to test this today. All thrusters were behaving as they should except for thrusters 7 and 8 which didn’t turn at all…
Is there another place in the code I need to specify this is for 8 thrusters?
Cheers,
Etienne
My best guess is this is because servo outputs 7 and 8 are specified as for the lights and camera tilt servo by default.
If you haven’t already you can try changing the lights and camera setup to use pins 9 and 10 instead, then go to the parameters and make sure SERVO_7_FUNCTION is set to Motor7, and similarly for 8.
Hi Eliot,
Many thanks for this. I usually load the default parameters for BR2 Heavy which change those values automatically.
All 8 thrusters seem to be behaving properly now.
I’ll be trying it in our tank on Monday.
Cheers,
E.
Hello Again,
I was able to give it a test today, thrusters seem to be responding as they should.
Having more issues:
Cheers,
E.
Did you build off ArduSub-stable, or ArduSub-beta / master?
Hmm, that’s odd, although if you’re using a beta or master branch firmware build it’s possible the parameters have changed, which may not be reflected in the version of QGC you’re using.
I am following this:
When I run this command
I get this:
edemers75@MSI:~/ardupilot$ git checkout ArduSub-stable -b new-branch
warning: unable to rmdir ‘modules/CrashDebug’: Directory not empty
warning: unable to rmdir ‘modules/DroneCAN/DSDL’: Directory not empty
warning: unable to rmdir ‘modules/DroneCAN/dronecan_dsdlc’: Directory not empty
warning: unable to rmdir ‘modules/DroneCAN/libcanard’: Directory not empty
warning: unable to rmdir ‘modules/DroneCAN/pydronecan’: Directory not empty
warning: unable to rmdir ‘modules/gsoap’: Directory not empty
M modules/ChibiOS
M modules/gbenchmark
M modules/gtest
M modules/mavlink
M modules/uavcan
M modules/waf
Switched to a new branch ‘new-branch’
Not sure if the warnings are important or not.
When I run this command
I get this:
edemers75@MSI:~/ardupilot$ git submodule update --init --recursive
Submodule ‘modules/libcanard’ (git://github.com/ArduPilot/libcanard.git) registered for path ‘modules/libcanard’
Cloning into ‘/home/edemers75/ardupilot/modules/libcanard’…
fatal: remote error:
The unauthenticated git protocol on port 9418 is no longer supported.
Please see Improving Git protocol security on GitHub - The GitHub Blog for more information.
fatal: clone of ‘git://github.com/ArduPilot/libcanard.git’ into submodule path ‘/home/edemers75/ardupilot/modules/libcanard’ failed
Failed to clone ‘modules/libcanard’. Retry scheduled
Cloning into ‘/home/edemers75/ardupilot/modules/libcanard’…
fatal: remote error:
The unauthenticated git protocol on port 9418 is no longer supported.
Please see Improving Git protocol security on GitHub - The GitHub Blog for more information.
fatal: clone of ‘git://github.com/ArduPilot/libcanard.git’ into submodule path ‘/home/edemers75/ardupilot/modules/libcanard’ failed
Failed to clone ‘modules/libcanard’ a second time, aborting
Not sure if after all these errors I am getting the right ardusub version.
Cheers,
E.
I am using the latest version from the link on your website for windows.
That shouldn’t be particularly problematic, it’s just some (unused) submodule folders that get left over from the branch switch, since master has some new submodules that aren’t in the Sub-4.0 branch (which the ArduSub-stable tag is currently sourced from). It’s best to avoid adding them to the tracked history and committing them, but even if you do accidentally do that they still wouldn’t have any code that actually tries to use them.
Ahh yes, that’s an issue that came up recently, and it’s an actual error, so it could stop things from working properly (in particular, because the submodules don’t get updated to track the branch, they would still be on the newer versions from master, which may have functionality differences). We unfortunately haven’t yet had the time to fix it properly, but that comment covers a workaround that can be used in the meantime.
Do you have a link so I can download the code manually?
I’ve zipped the repo at ArduSub-stable here, but it’s ~500MB to download, and because it was cloned directly from the ArduPilot repo you should be able to test with it but won’t be able to push changes to your own remote, or make pull requests.
That’s probably not particularly important though, since once you’ve confirmed your configuration works the way you want it to you can move the changes over to your own repo (off your master branch) and pull request from there
Hello again,
I downloaded the file and copied it to my ubuntu wsl via file explorer.
Now I get this when I try to compile:
Sorry for all the questions, I am not a linux guy and just feeling my way around.
Did you follow the build environment setup? That uses sudo internally, which I would expect should mean there shouldn’t be permission issues at this stage. If you have already installed the prerequisites then maybe try chmod +x waf, but it shouldn’t be necessary (waf should already be executable by default).
I did initially but do I need to do it again with the files you sent? Is waf stored inside ardupilot folders?
looks like I don’t have permission for anything. 
The master branch has different prerequisites to the ArduSub-stable tag.
Yes.
Can you try running with sudo? It should ask you to put your password in.
e.g. run
sudo Tools/environment_install/install-prereqs-ubuntu.sh -y