forked from PX4/PX4-Autopilot
-
Notifications
You must be signed in to change notification settings - Fork 15
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
Add 60k RPM support to voxl-esc driver #49
Merged
Changes from 19 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
4495820
init commit, version check working, ESC arm fails
modaljc 8d44d11
update packet size
modaljc 92aa25f
fix extended rpm packet packing function
modaljc 768a105
update data types for rpm packets
modaljc 1e9665d
replace magic number, clean up code
modaljc d53dc5d
clean up code
modaljc 917ea36
fix actuator tab, add voxl2_io default params
modaljc 7a7674e
Merge branch 'voxl-dev' of https://github.com/modalai/px4-firmware in…
modaljc e24a6a8
init commit, version check working, ESC arm fails
modaljc 1d829d9
update packet size
modaljc c162974
fix extended rpm packet packing function
modaljc 535047d
update data types for rpm packets
modaljc c5450e3
replace magic number, clean up code
modaljc 8304412
clean up code
modaljc 8c03952
fix actuator tab, add voxl2_io default params
modaljc e4d1d36
Merge branch '60k-RPM' of https://github.com/modalai/px4-firmware int…
modaljc 4d77eb7
Update packet packing functions
modaljc 9667d79
Comment out debug messages
modaljc 78b8130
Merge branch 'voxl-dev' of https://github.com/modalai/px4-firmware in…
modaljc dcb019e
Merge branch 'voxl-dev' of https://github.com/modalai/px4-firmware in…
modaljc 4d4732e
remove magic numbers, add rpm max variables
modaljc 3dd7d21
Update range checking, remove casting
modaljc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,11 @@ int VoxlEsc::init() | |
|
||
_uart_port = new VoxlEscSerial(); | ||
memset(&_esc_chans, 0x00, sizeof(_esc_chans)); | ||
for (int esc_id=0; esc_id < VOXL_ESC_OUTPUT_CHANNELS; ++esc_id){ | ||
_version_info[esc_id].sw_version = UINT16_MAX; | ||
_version_info[esc_id].hw_version = UINT16_MAX; | ||
_version_info[esc_id].id = esc_id; | ||
} | ||
|
||
//get_instance()->ScheduleOnInterval(10000); //100hz | ||
|
||
|
@@ -265,6 +270,20 @@ int VoxlEsc::flush_uart_rx() | |
return 0; | ||
} | ||
|
||
bool VoxlEsc::check_versions_updated(){ | ||
for (int esc_id=0; esc_id < VOXL_ESC_OUTPUT_CHANNELS; ++esc_id){ | ||
if (_version_info[esc_id].sw_version == UINT16_MAX) return false; | ||
} | ||
|
||
// PX4_INFO("Got all ESC Version info!"); | ||
_extended_rpm = true; | ||
_need_version_info = false; | ||
for (int esc_id=0; esc_id < VOXL_ESC_OUTPUT_CHANNELS; ++esc_id){ | ||
if (_version_info[esc_id].sw_version < VOXL_ESC_EXT_RPM) _extended_rpm = false; | ||
} | ||
return true; | ||
} | ||
|
||
int VoxlEsc::read_response(Command *out_cmd) | ||
{ | ||
px4_usleep(_current_cmd.resp_delay_us); | ||
|
@@ -302,7 +321,7 @@ int VoxlEsc::parse_response(uint8_t *buf, uint8_t len, bool print_feedback) | |
uint8_t packet_size = qc_esc_packet_get_size(&_fb_packet); | ||
|
||
if (packet_type == ESC_PACKET_TYPE_FB_RESPONSE && packet_size == sizeof(QC_ESC_FB_RESPONSE_V2)) { | ||
//PX4_INFO("Got feedback V2 packet!"); | ||
// PX4_INFO("Got feedback V2 packet!"); | ||
QC_ESC_FB_RESPONSE_V2 fb; | ||
memcpy(&fb, _fb_packet.buffer, packet_size); | ||
|
||
|
@@ -373,6 +392,11 @@ int VoxlEsc::parse_response(uint8_t *buf, uint8_t len, bool print_feedback) | |
else if (packet_type == ESC_PACKET_TYPE_VERSION_RESPONSE && packet_size == sizeof(QC_ESC_VERSION_INFO)) { | ||
QC_ESC_VERSION_INFO ver; | ||
memcpy(&ver, _fb_packet.buffer, packet_size); | ||
if (_need_version_info){ | ||
memcpy(&_version_info[ver.id], &ver, sizeof(QC_ESC_VERSION_INFO)); | ||
check_versions_updated(); | ||
break; | ||
} | ||
PX4_INFO("ESC ID: %i", ver.id); | ||
PX4_INFO("HW Version: %i", ver.hw_version); | ||
PX4_INFO("SW Version: %i", ver.sw_version); | ||
|
@@ -417,11 +441,11 @@ int VoxlEsc::parse_response(uint8_t *buf, uint8_t len, bool print_feedback) | |
switch (ret) { | ||
case ESC_ERROR_BAD_CHECKSUM: | ||
_rx_crc_error_count++; | ||
//PX4_INFO("BAD ESC packet checksum"); | ||
// PX4_INFO("BAD ESC packet checksum"); | ||
break; | ||
|
||
case ESC_ERROR_BAD_LENGTH: | ||
//PX4_INFO("BAD ESC packet length"); | ||
// PX4_INFO("BAD ESC packet length"); | ||
break; | ||
} | ||
} | ||
|
@@ -652,7 +676,8 @@ int VoxlEsc::custom_command(int argc, char *argv[]) | |
0, | ||
id_fb, | ||
cmd.buf, | ||
sizeof(cmd.buf)); | ||
sizeof(cmd.buf), | ||
get_instance()->_extended_rpm); | ||
|
||
cmd.response = true; | ||
cmd.repeats = repeat_count; | ||
|
@@ -1038,6 +1063,12 @@ bool VoxlEsc::updateOutputs(bool stop_motors, uint16_t outputs[MAX_ACTUATORS], | |
_esc_chans[i].rate_req = 0; | ||
|
||
} else { | ||
if (_extended_rpm) { | ||
if (outputs[i] > 65530) outputs[i] = 65530; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps a constant definition of 65530 and 32766 with some comments about why those values were chosen There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will make this happen |
||
} else { | ||
if (outputs[i] > 32766) outputs[i] = 32766; | ||
} | ||
|
||
if (!_turtle_mode_en) { | ||
_esc_chans[i].rate_req = outputs[i] * _output_map[i].direction; | ||
|
||
|
@@ -1047,20 +1078,21 @@ bool VoxlEsc::updateOutputs(bool stop_motors, uint16_t outputs[MAX_ACTUATORS], | |
} | ||
} | ||
} | ||
|
||
|
||
Command cmd; | ||
cmd.len = qc_esc_create_rpm_packet4_fb(_esc_chans[0].rate_req, | ||
_esc_chans[1].rate_req, | ||
_esc_chans[2].rate_req, | ||
_esc_chans[3].rate_req, | ||
_esc_chans[0].led, | ||
_esc_chans[1].led, | ||
_esc_chans[2].led, | ||
_esc_chans[3].led, | ||
_fb_idx, | ||
cmd.buf, | ||
sizeof(cmd.buf)); | ||
|
||
_esc_chans[1].rate_req, | ||
_esc_chans[2].rate_req, | ||
_esc_chans[3].rate_req, | ||
_esc_chans[0].led, | ||
_esc_chans[1].led, | ||
_esc_chans[2].led, | ||
_esc_chans[3].led, | ||
_fb_idx, | ||
cmd.buf, | ||
sizeof(cmd.buf), | ||
_extended_rpm); | ||
|
||
if (_uart_port->uart_write(cmd.buf, cmd.len) != cmd.len) { | ||
PX4_ERR("Failed to send packet"); | ||
|
@@ -1148,6 +1180,19 @@ void VoxlEsc::Run() | |
} | ||
} | ||
|
||
/* Get ESC FW version info */ | ||
if (_need_version_info){ | ||
for (uint8_t esc_id=0; esc_id < VOXL_ESC_OUTPUT_CHANNELS; ++esc_id){ | ||
Command cmd; | ||
cmd.len = qc_esc_create_version_request_packet(esc_id, cmd.buf, sizeof(cmd.buf)); | ||
if (_uart_port->uart_write(cmd.buf, cmd.len) == cmd.len) { | ||
if (read_response(&_current_cmd) != 0) PX4_ERR("Failed to parse version request response packet!"); | ||
} else { | ||
PX4_ERR("Failed to send version request packet!"); | ||
} | ||
} | ||
} | ||
|
||
_mixing_output.update(); //calls MixingOutput::limitAndUpdateOutputs which calls updateOutputs in this module | ||
|
||
/* update output status if armed */ | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is confusing to me. The comment says it is trying to prevent overflow of int16_t. So why would UINT16_MAX-5 accomplish that?
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.
When using extended range rpm command we divide the value passed in by 2 before sending to the ESC. Requested rpms passed in can theoretically be larger than
UINT16_MAX
since they are int32_t (RPMs can be negative), which would mean that it's possible when we divide the value for extended range packet that it is still larger than UINT16_MAX, which would cause overflow when assigning the value to the data array (int16_t). I.e. if a value of 2,000,000 is passed in, the divided value is 1,000,000 which is still aboveUINT16_MAX
but when assigned to the int16_t data variable it would be truncated to 16960.After thinking about it more, I think it might be a good idea to add some range limiting to the negative end of the range as well as maybe limit the standard range too since the input is int32_t... or we could leave it up to the user to input the correct values... thoughts?
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.
Let's let Alex submit his comments to see what his ideas are.
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.
From Alex:
in the limit values for regular (not extended rpm) the limits should be +-/32k (2^15), not UINT16_MAX . For the extended rpm range, the limit will be +/- 65K (but lets use slightly smaller limit as you already started doing)
I updated this in my latest commit (3dd7d21). I added max and min for both the standard and extended range... -32k to 32k and -65k to 65k, respectively