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

SCALED_IMU and SCALED_PRESSURE implementations prevent templating the message streams in PX4/Firmware. #949

Closed
mcsauder opened this issue Jul 14, 2018 · 2 comments
Labels

Comments

@mcsauder
Copy link

Hi there,

The files mavlink_msg_scaled_imu2.h, mavlink_msg_scaled_imu3.h, are copy/paste instances of mavlink_msg_scaled_imu.h. Similarly, the files mavlink_msg_scaled_pressure2.h, mavlink_msg_scaled_pressure3.h are also similar copy/paste instances of mavlink_msg_scaled_pressure.h. The only material difference between the files is respective numbering.

Because the typedef'd structs and methods in the 2nd and 3rd instances of these files do not inherit from the primary instance, and because they are uniquely identified as "2" and "3" throughout the files, it is difficult/impossible to utilize templating when dealing with these classes at higher levels. The unfortunate result of the implementation as it stands leads to additional copy/paste bloat elsewhere in codes bases utilizing mavlink. It also feels like code bloat that could be eliminated

Although these message files are functional, perhaps they could be improved. The files mavlink_msg_actuator_control_target.h and mavlink_msg_servo_output_raw.h are examples of how the scaled_imu and scaled_pressure message implementations could exist to allow multiple instances of the messages in a templated structure.

Possible solutions might include:

  1. Deprecate the second and third instances of these files and add an index field to the message definitions, or
  2. Refactor the second and third file instances to inherit from the first instance.

Refactoring the current implementation could allow a templated version of this PX4 merge request.

Let me know any advice or solutions you might have to offer. Thanks.

-Mark

@stale
Copy link

stale bot commented Apr 23, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Apr 23, 2020
@hamishwillee
Copy link
Collaborator

FWIW Since all of this is autogenerated from the messages and from the generator perspective there is no connection between them I don't see how this can be restructured.

The right way to fix this would have been to add an index field and deprecate the other messages. Given the widespread use of the messages I'm not sure that the change would be considered worth the cost.

I'm closing now. If you want to add index, deprecate the other messages, create a PR and we can discuss in the dev call.

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

No branches or pull requests

2 participants