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

Command "iso_wait_for_stop" now works as intended in EvManager #800

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

SebaLukas
Copy link
Contributor

Describe your changes

The "iso_wait_for_stop" implementation was not correct.

Issue ticket number and link

r_ev[0]->call_stop_charging(); was not called

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

if (!sim_data.sleep_ticks_left.has_value()) {
sim_data.sleep_ticks_left =
std::stoll(arguments[0]) * static_cast<long>(1 / static_cast<float>(loop_interval_ms)) + 1;
if (not sim_data.sleep_ticks_left.has_value()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not introduced in this PR, but std::stoll et al are used without catching exceptions here and in other places in EVManager. We should add try catch everywhere probably in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I'll plan that.

Copy link
Member

@MarzellT MarzellT left a comment

Choose a reason for hiding this comment

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

Looks good to me

@SebaLukas SebaLukas merged commit 4f90a84 into main Jul 29, 2024
8 checks passed
@SebaLukas SebaLukas deleted the fix/ev_manager_sleep branch July 29, 2024 07:24
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.

3 participants