-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
drivers: add support for TeraRanger Evo #7962
Conversation
This commit adds i2c support for TeraRanger Evo sensor by Terabee Signed-off-by: Mateusz Sadowski <msadowski90@gmail.com>
src/drivers/trevo/CMakeLists.txt
Outdated
@@ -0,0 +1,43 @@ | |||
############################################################################ | |||
# | |||
# Copyright (c) 2015 PX4 Development Team. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update the year please
src/modules/sensors/sensor_params.c
Outdated
@@ -1088,6 +1088,16 @@ PARAM_DEFINE_INT32(SENS_EN_SF0X, 0); | |||
PARAM_DEFINE_INT32(SENS_EN_MB12XX, 0); | |||
|
|||
/** | |||
* TeraRanger Evo (trevo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a TeraRanger sensor, I think it would be better to have a parameter SENS_EN_TR
and add the option to set to 0, if none, 1 if TeraRanger One, 2 if TeraRanger Evo.
src/drivers/trevo/trevo.cpp
Outdated
@@ -0,0 +1,938 @@ | |||
/**************************************************************************** | |||
* | |||
* Copyright (c) 2013-2015 PX4 Development Team. All rights reserved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
year
src/drivers/trevo/trevo.cpp
Outdated
/* Configuration Constants */ | ||
#define TREVO_BUS PX4_I2C_BUS_EXPANSION | ||
#define TREVO_BASEADDR 0x31 /* 7-bit address */ | ||
#define TREVO_DEVICE_PATH "/dev/trevo" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix indentation please
src/drivers/trevo/trevo.cpp
Outdated
|
||
/* TREVO Registers addresses */ | ||
|
||
#define TREVO_MEASURE_REG 0x00 /* Measure range register */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix indentation please
src/drivers/trevo/trevo.cpp
Outdated
virtual int init(); | ||
|
||
virtual ssize_t read(struct file *filp, char *buffer, size_t buflen); | ||
virtual int ioctl(struct file *filp, int cmd, unsigned long arg); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix indentation please
src/drivers/trevo/trevo.cpp
Outdated
ringbuffer::RingBuffer *_reports; | ||
bool _sensor_ok; | ||
uint8_t _valid; | ||
int _measure_ticks; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix indentation please
src/drivers/trevo/trevo.cpp
Outdated
* @param address The I2C bus address to probe. | ||
* @return True if the device is present. | ||
*/ | ||
int probe_address(uint8_t address); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix indentation please
src/drivers/trevo/trevo.cpp
Outdated
* Perform a poll cycle; collect from the previous measurement | ||
* and start a new one. | ||
*/ | ||
void cycle(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix indentation please
Awesome! Good to go if you could address the comments from @TSC21 |
@msadowski CI is failing on format check so you should address that by fixing style. |
The only difference between the TR One and TR Evo drivers are the base address and the min/max distances, right? What do you think about having only one driver that works for both? Either the user chooses which device they want to use it with via parameter (as per @TSC21's suggestion) or we auto-detect which sensor is connected by probing the I2C base addresses. |
@ndepal that seems an awesome suggestion. @msadowski can you address this instead IOT simplify the driver implementation? |
When I was testing out the Evo a while back I actually did that. I never got around to testing that the TR One still works with that change and therefore didn't end up making a PR. You can see the implementation here: ndepal@85483c2 One problem might be if there are other devices with the same I2C address. E.g. an Evo and another device with the TR One address. The teraranger driver might then end up thinking that there is a TR One. One way to work around that would be to have the parameter be 0 = off, 1 = autodetect, 2 = TR One, 3 = TR Evo, but I'm not sure if it's worth doing that, it might not be a real issue. |
👍 |
@ndepal thanks for your contribution, I can test it tomorrow and give you feedback on it. We were actually discussing within the team about having one or more drivers. One driver is obviously cleaner but what I'm worrying about are cases when someone would like to use both TeraRanger One and Evo (for example one of them pointing forward). @TSC21 If you don't think it's likely to be an issue for anybody I'm more than happy to test @ndepal implementation and if needed fix it and apply it. |
I'm more than happy that you do it. Thanks |
This reverts commit d1da112.
This commit changes old trone driver into a generic TeraRanger driver that supports both TeraRanger One and TeraRanger Evo. As a part of the change a new parameter was created SENS_EN_TRANGER that allows to specify the following modes of operation: 0 - sensors disabled 1 - autodetect sensors 2 - use TeraRanger One rangefinder 3 - use TeraRanger Evo rangefinder Signed-off-by: Mateusz Sadowski <msadowski90@gmail.com>
Signed-off-by: Mateusz Sadowski <msadowski90@gmail.com>
I updated the driver to take the 4 parameters (disabled, automatic, One, Evo), fixed up the copyright dates and tested it with both TeraRanger One and Evo. Everything seems to work now. Please let me know if there is anything you would like us to refactor further. |
Great! Looks good to me. |
@msadowski awesome work! BTW any plans to add a POSIX driver to the TeraRanger products? |
@TSC21 POSIX driver would be truly neat, I'll ask around to see what others think. I will make sure to record it in our backlog as well. |
@TSC21: quick follow up: could you please tell me what benefits do you see as a user to have the POSIX driver? |
You have different platforms that can there run it - RPi+Navio, Bebop, OcPoC... Or any linux system running PX4 POSIX. |
Terabee is about to release a new range finder sensor. This commit adds a driver for it.
Any feedback is welcome and highly appreciated.