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 StoreTelemetry function for hils test #69

Merged
merged 2 commits into from
Mar 28, 2022

Conversation

seki-hiro
Copy link
Member

Overview

Add StoreTelemetry function for I2C HILS test.

Issue

  • Related issues

Details

In order to store some frames of telemetry in the USB-I2C target converter in advance, StoreTelemetry function is added.

Validation results

Link to tests or validation results.

Scope of influence

eg. The behavior of XX will be change.

Supplement

This function may be useful for users when they perform I2C HILS test.

Note

  • If there are related Projects, tie them together.
  • Assignees should be set if possible.
  • Reviewers should be set if possible.
  • Set priority label if possible.

@seki-hiro seki-hiro requested a review from 200km March 24, 2022 07:44
@seki-hiro seki-hiro self-assigned this Mar 24, 2022
Copy link
Member

@200km 200km left a comment

Choose a reason for hiding this comment

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

小さなコメントなのでapproveします。

@@ -149,3 +149,16 @@ int ObcI2cTargetCommunicationBase::GetStoredFrameCounter() {
if (sim_mode_ != OBC_COM_UART_MODE::HILS) return -1;
return hils_port_manager_->I2cTargetGetStoredFrameCounter(hils_port_id_);
}

int ObcI2cTargetCommunicationBase::StoreTelemetry(
const unsigned int stored_frame_num, const unsigned int tlm_size) {
Copy link
Member

Choose a reason for hiding this comment

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

[NITS] ヘッダと同じような改行のほうが個人的には好みなんですが、そうすると1行あたりが長くなってしまうという感じですか?

Copy link
Member Author

Choose a reason for hiding this comment

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

はい、元々はヘッダと同じようにしていたのですが、formatのチェックを通らなかったので、このコミットで変更を加えました。

変更前は1行87文字だったのですが、formatでは1行80文字までなのかなという認識をしています。

Copy link
Member

Choose a reason for hiding this comment

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

@sksat 一行の制約を100文字くらいにするのはやめたほうが良いですかね?
略語無しで行こうという方針なので、結構長い変数名などが多く、文字数制限にすぐひっかかり逆に読みづらく感じることもあるのですが、、、

Copy link
Member

Choose a reason for hiding this comment

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

@seki-hiro フォーマットについては別途議論ということで、こちらは一旦マージしてしまって良いです。

Copy link
Member Author

Choose a reason for hiding this comment

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

承知しました、マージします。

@seki-hiro seki-hiro merged commit b4b6350 into develop Mar 28, 2022
@seki-hiro seki-hiro deleted the feature/add_StoreTlm_function_for_hils branch March 28, 2022 10:43
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