AP_Scripting: mount-djirs2 script handles latest gimbal software #25692
Conversation
|
I'll look into this later this week. Thanks for getting back to it.
I don't think this is true? In all the marketing material and documentation and menus, the "normal" orientation of the gimbal has the camera above the handle and control unit. In most drone applications the camera will hang below the handle and control unit, which is why we provide the upside down mode, correct? As for the roll/pitch being reversed, I'm not sure what's up. The code to retrieve the data as it's written directly contradicts the documentation at the top of the file and the spec available here. But if it works for you there must be some gimbals which are reversed? I'll double check this too. Might be best to just make everyone update. |
|
Txs for the feedback and offer of testing. You may be right about the definition of "upside-down". I'll check with another user I know about what he thinks is rightside-up vs upside-down. |
|
Regarding the testing script in the branch in the old issue, I get The script in this branch works fine with my gimbal. But the roll and pitch axis reports are for sure swapped. If I set Again, the protocol documentation contained within the script itself and the PDF from DJI suggests the interpretation my gimbal uses is correct and the script code is wrong. You say your gimbal moves correctly, did you check that the reported data is in fact correct using the debug option? Can you remind which firmware your gimbal is on? Mine is 01.05.00.20. |
DJI R SDK version 2.2.0.5 released on October 30, 2020 added CmdSet and CmdID bytes to reply frames before the data segment which need to be skipped when parsing replies. Tested with gimbal firmware 01.04.00.20 and 01.05.00.20 (latest version).
rmackay9
force-pushed
the
dji-mount-fix
branch
from
5ce8479
to
2c1d197
Compare
|
Thanks for the testing and feedback. As hard as it is to believe, I think DJI had a bug in the angle reporting for roll and pitch in the legacy firmware (they were swapped). I've updated the PR so that for the "latest" DJI firmware it uses the correct order. Maybe you could comment whether you're happy with these changes? That'll clear the way for merging I think. Txs again. |
| local pitch_field = 17 | ||
| local roll_field = 19 |
There was a problem hiding this comment.
Would like to see a comment here that this field ordering contradicts the spec but is how an actual gimbal of version XX (whichever version yours is) is observed to behave.
|
I personally would like to see support for the legacy protocol dropped completely. Updating is slow but not hard; the user has to have hooked up the app to activate the gimbal in the first place. We are also assuming whichever version updated the packet length is whichever version fixed the reporting bug, which is hard to determine. Aside from that comment nit, I'm happy with these changes if you're happy with the additional complications of supporting multiple versions. |
|
I think it's so easy for us to keep supporting the old firmware that we should. I agree re the comment but I'll do that in the future I think in order to get this in now that it's functionally correct. |
|
@tpwrules Randy has committed to adding coimments on the roll/pitch issue in a follow-up PR. We discussed removing legacy protocol support but Randy made a spirited argument that in this case it's a very small amount of work to support the older firmware and there's no other drawback in doing so. We do enforce minimum firmware version on some camera drivers, but in this case we don't need to - yet! Thanks for looking through this PR, we do appreciate the reviews :-) |
This is a replacement for PR #24461
Three important fixes are included:
This has been tested on a DJIR2 running the legacy firmware. I hope that @tpwrules can check that it also works on the latest firmware.