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

Add CAN beep #14712

Closed
wants to merge 7 commits into from
Closed

Add CAN beep #14712

wants to merge 7 commits into from

Conversation

CUAVcaijie
Copy link
Contributor

Add CAN buzzer

@TSC21 TSC21 requested a review from dagar April 20, 2020 09:34
@@ -0,0 +1,106 @@
/****************************************************************************
*
* Copyright (c) 2014, 2015 PX4 Development Team. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Copyright (c) 2014, 2015 PX4 Development Team. All rights reserved.
* Copyright (c) 2020 PX4 Development Team. All rights reserved.

UavcanUavcanBeep::init()
{
/*
* Setup timer and call back function for periodic updates
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this description to the header file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I will modify

@@ -0,0 +1,91 @@
/****************************************************************************
*
* Copyright (c) 2014, 2015 PX4 Development Team. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Copyright (c) 2014, 2015 PX4 Development Team. All rights reserved.
* Copyright (c) 2020 PX4 Development Team. All rights reserved.


/**
* @file beep.hpp
*
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a description here with @brief?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

/**
* @brief The UavcanUavcanBeep class
*/

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change

/*
* Max update rate to avoid exessive bus traffic
*/
static constexpr unsigned MAX_RATE_HZ = 100;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
static constexpr unsigned MAX_RATE_HZ = 100;
static constexpr unsigned MAX_RATE_HZ = 100;

uORB::Subscription _tune_control_sub{ORB_ID(tune_control)};

/*
* libuavcan related things
Copy link
Member

Choose a reason for hiding this comment

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

Can you replace with details on what each thing is doing instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

*/
uavcan::Publisher<uavcan::equipment::indication::BeepCommand> _beep_pub;

uavcan::TimerEventForwarder<TimerCbBinder> _timer;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uavcan::TimerEventForwarder<TimerCbBinder> _timer;
uavcan::TimerEventForwarder<TimerCbBinder> _timer;

@@ -130,6 +130,7 @@ px4_add_module(
sensors/flow.cpp
sensors/gnss.cpp
sensors/mag.cpp
sensors/beep.cpp
Copy link
Member

Choose a reason for hiding this comment

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

Can you please move this above and make sure it is alphabetically ordered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@stale
Copy link

stale bot commented Aug 24, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale label Aug 24, 2020
@CUAVcaijie CUAVcaijie closed this Jan 21, 2021
@CUAVcaijie CUAVcaijie deleted the can_beep branch January 21, 2021 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants