ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
11.08k stars 17.65k forks source link

VectorNav Driver Improvements #27151

Open lashVN opened 6 months ago

lashVN commented 6 months ago

I apologize for the vague title, I have a few separate feature improvements for the driver in this ticket and am happy to split them to individual tickets based on feedback.

Feature request

  1. Expand usability beyond VN-100 and VN-300
    • The driver can generally work with any VN sensor, it's largely this model_name check that needs simple expansion to adapt to any VN sensor
      1. Change TYPE enum to "AHRS" or "INS", which directs the use of which packet to send and messages to publish
      2. new TYPE enum should be set by model string, allowing for 110, 210, 310, 100, 200, 300
      3. Will have to add VN_IMU_RATE uint16_t to handle the case that VN-300 is 400, where the rest are 800. This should be set by the model string.
  2. Add response validation of commands sent to the VectorNav
    • I've experienced a few bugs where VN responds with VNERR for a handful of reasons, which is not caught by AP and makes debugging more difficult.
      1. Expand wait_register_response to accept arbitrary std::string as a command so that I can send either RRG or WRG.
      2. Update decode_latest_term to make generic
  3. Add INS Status as an output in VN_Packet_1, and check the Mode information before publishing state data
    • VectorNav INS all switches fundamental state based on the InsStatus.Mode, so this should at minimum be logged for debugging. Ideally, we prevent the publishing of some states based on VN-reported validity.
      1. Edit wrg,75 command to include InsStatus
      2. Add InsStatus to vn_pkt1_header and VN_packet1, and update VN_PKT1_LENGTH
      3. Check INS Status > 0 (or maybe > 1) before publishing positions and velocities, and corresponding validity flags
        • This warrants some discussion on desired behavior, what validity warrants publishing data, and if there is any other AP state flags we can set
      4. Add INS Status to EAH1 logging message
  4. Set binary output message to only active port
    • We should only be editing the messages on the active port, allowing the user to use the other port for their own purposes. I've seen a few system failures due to this issue.
      1. If (3) has already been done, we can use the new wait_register_response to poll the active serial using "VNRRG,6,0", save the response in a new uint8_t VN_SERIAL_NUM, and use it for the bin msg WRGs
  5. Remove preflight check on GPS 2 to report a fix before allowing flight
    • This point warrants some discussion about desired driver behavior. The VN-300 (and VN-310) driver can reasonably operate with only 1 GNSS and reach a high-quality heading using dynamic alignment.

Is your feature request related to a problem? Please describe.

Describe the solution you'd like

Describe alternatives you've considered

Platform [ ] All [ ] AntennaTracker [ x] Copter [ x] Plane [ ] Rover [ x] Submarine

Additional context

lashVN commented 6 months ago

@tridge You and I have discussed some of these improvements a while ago, and there's now more users interested in increasing the driver robustness. What are your thoughts on these changes? I'm happy to get the work done and it sounds like I may have the ability to test these. Please let me know your thoughts on the points and any possible issues as we look toward making these improvements

Also, let me know if you'd prefer these changes made in one big feature branch or split up.

IamPete1 commented 6 months ago

Also, let me know if you'd prefer these changes made in one big feature branch or split up.

Several small and easy to review PRs would be my preference.

peterbarker commented 6 days ago

@lashVN this issue predates several of your PRs to master.

Could you review this issue to see what's resolved and what's remaining, please?