Skip to content

Thrusters control v1.0 #14

Open
WaspishRaccoon wants to merge 2 commits intomainfrom
thrusters_BFSDRK
Open

Thrusters control v1.0 #14
WaspishRaccoon wants to merge 2 commits intomainfrom
thrusters_BFSDRK

Conversation

@WaspishRaccoon
Copy link
Copy Markdown
Collaborator

Added hydrv_thrusters_control class v1.0 with dependencies


public:
constexpr Thruster(unsigned thruster_tim_channel,
hydrv::timer::TimerLow &thruster_tim,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Вот это менять не стоило, пусть остаётся ссылками

hydrolib::controlling::ThrustGenerator<THRUSTERS_COUNT> thrust_generator_;

private:
static_assert(hydrolib::controlling::ThrusterConcept<ThrusterControl>,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Не уверен, что это следует писать внутри класса

class ThrusterControl
{
private:
int x_rotation_gain_[THRUSTERS_COUNT];
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

А зачем это хранить здесь? Почему бы не пробросить это сразу в thrust_generator?

int y_linear_gain_[THRUSTERS_COUNT];
int z_linear_gain_[THRUSTERS_COUNT];

int dest_[THRUSTERS_COUNT];
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Почему бы эту переменную не сделать временной внутри функции обработки?

{
for (int i = 0; i < THRUSTERS_COUNT; i++)
{
thrusters_[i](thruster_tim_channel[i], *thruster_tim[i],
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Вот это вообще интересная штука, не знал, что так можно. Это точно компилируется?

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.

2 participants