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

CAN: added CAN_Dn_PROTOCOL2 for 11 bit auxillary protocol #25617

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

tridge
Copy link
Contributor

@tridge tridge commented Nov 23, 2023

This allows for a 2nd CAN protocol specified by CAN_Dn_PROTOCOL2. This protocol needs to use 11 bit addressing and the same CAN bitrate as the DroneCAN driver (usually 1MBit)
With this enabled users no longer need to dedicate a CAN bus for devices such as Benewake CAN lidar, USD1 radar and various EFI CAN ECUs
Tested with both SITL and a CubeOrangePlus with a Benewake Lidar

@tridge tridge added the CAN label Nov 23, 2023
@tridge tridge added the WikiNeeded needs wiki update label Nov 23, 2023
@IamPete1
Copy link
Member

IamPete1 commented Nov 23, 2023

If its DroneCAN only shouldn't the param live down a level with the DroneCAN params?

It would be nice to bypass the 11 bit limit for scripting so it can be used to add support for new/bespoke DroneCAN messages.

This will be very useful.

@tridge
Copy link
Contributor Author

tridge commented Nov 23, 2023

If its DroneCAN only shouldn't the param live down a level with the DroneCAN params?

It is only DroneCAN at the moment, but in principle other protocols (eg. Piccolocan) could implement this by implementing the add_11bit_driver() method.
Structurally, this is principally a AP_CANManager feature, and from the point of view of most end-users using DroneCAN they are adding a 2nd protocol. The restriction of only 11bit comes from not being able to reliably distinguish protocols that do the same addressing scheme.

It would be nice to bypass the 11 bit limit for scripting so it can be used to add support for new/bespoke DroneCAN messages.

that would be nice, but we'd need to work out how to do that. I suspect we'd need to be able to pass a list of message IDs we want to get in scripting so we don't get all messages (similar to the filter list in MAVCAN). Then we'd need to make the DSDL compiler generate lua bindings. That is quite a large project.

@IamPete1
Copy link
Member

that would be nice, but we'd need to work out how to do that. I suspect we'd need to be able to pass a list of message IDs we want to get in scripting so we don't get all messages (similar to the filter list in MAVCAN). Then we'd need to make the DSDL compiler generate lua bindings. That is quite a large project.

I think to start with a virtual mirror would be OK, just as if you had two physical CAN ports connected to the same bus. That is the current solution to the issue. Fancy filters would be nice, but they only apply to DroneCAN.

@IamPete1
Copy link
Member

Also once this is supported for more protocols you might want the 29 bit protocol as the auxiliary and the 11 bit as the primary.

@tridge
Copy link
Contributor Author

tridge commented Nov 27, 2023

Also once this is supported for more protocols you might want the 29 bit protocol as the auxiliary and the 11 bit as the primary.

that would be possible of course, but can you give an example of a use case?

@@ -51,6 +51,14 @@ const AP_Param::GroupInfo AP_CANManager::CANDriver_Params::var_info[] = {
AP_SUBGROUPPTR(_piccolocan, "PC_", 5, AP_CANManager::CANDriver_Params, AP_PiccoloCAN),
#endif

// @Param: PROTOCOL2
// @DisplayName: Secondary protocol with 11 bit CAN addressing
// @Description: Secondary protocol with 11 bit CAN addressing
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should add a comment to say that PROTOCOL should be set to DroneCAN for this to work.

Copy link
Contributor

@rmackay9 rmackay9 left a comment

Choose a reason for hiding this comment

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

Not my area really but it looks ok to me.

@tridge tridge merged commit ad59f6d into ArduPilot:master Nov 28, 2023
87 checks passed
@EosBandi
Copy link
Contributor

EosBandi commented Dec 4, 2023

@tridge, Can we add an exception for scripting drivers as secondary driver, and instead of using 11bit addressing, we can pass raw messages? This allows the use of DroneCAN devices from scripting without connecting the two buses together. It will work well with this #25703

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CAN WikiNeeded needs wiki update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants