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

Feature/sw 48 can servo feature addition #97

Merged
merged 1 commit into from
Feb 11, 2024

Conversation

Pradeep-Carbonix
Copy link
Contributor

  1. Update uavcan.equipment.power.CircuitStatus
  2. update uavcan.equipment.device.Temperature

@Pradeep-Carbonix Pradeep-Carbonix requested review from robertlong13, Jono453 and a user January 19, 2024 04:12
Copy link
Collaborator

@robertlong13 robertlong13 left a comment

Choose a reason for hiding this comment

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

We should extend the onboard log message instead of adding new messages. If you check the CSRV message in master, it has voltage, current, and temperatures now. We should match master's message.

1. Update uavcan.equipment.power.CircuitStatus
   - Voltage, Current and Error_flags
2. update uavcan.equipment.device.Temperature
   - Temperature and Error_flags
@Pradeep-Carbonix Pradeep-Carbonix force-pushed the feature/SW-48-can-servo-feature-addition branch from e1e5c4f to 9cfda82 Compare January 25, 2024 02:23
@Pradeep-Carbonix
Copy link
Contributor Author

We should extend the onboard log message instead of adding new messages. If you check the CSRV message in master, it has voltage, current, and temperatures now. We should match master's message.

  • Telemetry on MKS servos are sent as different messages. This needs to be handled separately:
    • uavcan.equipment.power.CircuitStatus
    • uavcan.equipment.device.Temperature

Thanks for suggesting to make these messages independent. The new generic names will not limit them to only mean servo/actuator logs

@Pradeep-Carbonix Pradeep-Carbonix self-assigned this Jan 25, 2024
@robertlong13 robertlong13 self-requested a review January 25, 2024 05:07
@loki077
Copy link
Contributor

loki077 commented Feb 3, 2024

For tracking
These are the below 2 CAN packet we are trying to log:
image
image

For future reference:
There is a different method of the temperature logging implementation ArduPilot/ardupilot#25993 but as it will make a lot of code change to merge this we will stick to our current implementation for now.

Copy link
Contributor

@loki077 loki077 left a comment

Choose a reason for hiding this comment

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

Code structure looks good. Just please confirm in the comments it is been tested on a hardware as I don't have to test on. Also, @Pradeep-Carbonix if you have any test report put a link here which will be good for reference.

@loki077
Copy link
Contributor

loki077 commented Feb 11, 2024

@robertlong13 do you have any comments on this PR? or good to go ahead and merge?

@robertlong13 robertlong13 merged commit 1173498 into CxPilot Feb 11, 2024
62 checks passed
@Pradeep-Carbonix
Copy link
Contributor Author

Check the logs here : \Dropbox (Carbonix Company)\Carbonix Engineering Team Folder\07_Test_Results\MKS HBL3850

@robertlong13 robertlong13 deleted the feature/SW-48-can-servo-feature-addition branch June 17, 2024 07:06
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.

4 participants