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

Detect external buses on FMU variants correctly and clean up driver implementation #8300

Merged
merged 7 commits into from
Nov 20, 2017

Conversation

dagar
Copy link
Member

@dagar dagar commented Nov 16, 2017

  • merge posix and nuttx Device and CDev
  • create px4_i2c_bus_external/px4_spi_bus_external helpers
    • implement special handling for FMUv3 (all i2c external, all spi internal)
  • push per driver external checks into Device class

@LorenzMeier
Copy link
Member

@dagar I'm currently testing this.

@dagar dagar force-pushed the pr-drivers_external branch from 33c9347 to 2b87eb4 Compare November 18, 2017 03:46
@LorenzMeier
Copy link
Member

Compile error:

In file included from /Users/distiller/Firmware/src/drivers/device/device.h:37:
/Users/distiller/Firmware/src/drivers/device/CDev.hpp:110:18: fatal error: 
      'device::CDev::read' hides overloaded virtual function
      [-Woverloaded-virtual]
        virtual ssize_t read(file_t *filep, char *buffer, size_t buflen);
                        ^
/Users/distiller/Firmware/src/drivers/device/Device.hpp:96:14: note: hidden
      overloaded virtual function 'device::Device::read' declared here: type
      mismatch at 1st parameter ('unsigned int' vs 'device::file_t *')
        virtual int     read(unsigned address, void *data, unsigned coun...
                        ^

@dagar dagar force-pushed the pr-drivers_external branch 2 times, most recently from a52616f to 61a3df6 Compare November 18, 2017 17:04
@dagar dagar force-pushed the pr-drivers_external branch from 61a3df6 to fc49bb1 Compare November 18, 2017 17:32
@dagar dagar force-pushed the pr-drivers_external branch from c23ffe3 to 38dfb7d Compare November 18, 2017 18:38
Copy link
Member

@LorenzMeier LorenzMeier left a comment

Choose a reason for hiding this comment

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

I've reviewed the code and ran it successfully on my setup. I have inspected the mag reports from the different mags.

Still needs flight testing but is looking good.

@LorenzMeier LorenzMeier changed the title [WIP] drivers px4_i2c_bus_external/px4_spi_bus_external Detect external buses on FMU variants correctly and clean up driver implementation Nov 18, 2017
@LorenzMeier
Copy link
Member

@PX4TestFlights @mrpollo Please add this to the test queue.

@dagar
Copy link
Member Author

dagar commented Nov 20, 2017

I've verified a few different pixhawk 2.1 configurations with different mags connected to different buses and different board and external mag rotations. Things seem to be correctly marked as external now, but there are still minor issues.

For example the hmc5883 i2c rotation supplied here is not correct for an external device. https://github.com/PX4/Firmware/blob/master/ROMFS/px4fmu_common/init.d/rc.sensors#L77

The original pixhawk 2 cube also needs to be reviewed.

@dagar
Copy link
Member Author

dagar commented Nov 20, 2017

Here's a pixhawk 2.1 cube with current master. The pixhawk is rotated roll 180, and there's an hmc5883 mag connected to the GPS 2 port (aka "internal" i2c).

ph2_orig_roll180

@LorenzMeier
Copy link
Member

Do you feel you can finish rotations and I can focus on just validating them?

@dagar
Copy link
Member Author

dagar commented Nov 20, 2017

Yes, and I think we're nearly done, but one question is how to handle the situation where a rotation is supplied on the command line for an external mag. https://github.com/PX4/Firmware/blob/master/ROMFS/px4fmu_common/init.d/rc.sensors#L77

Should the driver ignore that command line for an external or do we work around it in rc.sensors?

@LorenzMeier
Copy link
Member

Just ignore for an external? I'm also fine with working around it for now.

@r0gelion
Copy link
Contributor

@LorenzMeier we don't have a Pixhawk 2.1 to test.

@dagar
Copy link
Member Author

dagar commented Nov 20, 2017

@r0gelion testing other FMUv3 variants would help as well.

@LorenzMeier LorenzMeier merged commit 00a47ba into master Nov 20, 2017
@dagar dagar deleted the pr-drivers_external branch November 21, 2017 20:28
@msadowski
Copy link
Contributor

@dagar I just tested this fix with i2c rangefinder (TeraRanger Evo) on Pixhawk 2.1 and couldn't make it work. Are there any extra steps that have to be performed to make it work?

@dagar
Copy link
Member Author

dagar commented Dec 6, 2017

@msadowski can you try the other i2c bus? Please open a new issue if you can't get it working.

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.

5 participants