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 power consumption - v2 #17346

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eyusupov
Copy link
Contributor

@eyusupov eyusupov commented Apr 8, 2024

Summary

Reworked #17329 with the reduced amount of code.

Test Plan

Single battery when charging
Single battery when discharging

Additional Information

@thiagoftsm
Copy link
Contributor

Hello @eyusupov,

Now that other PR is merged, please, rebase this PR, and we will have conditions to proceed with it.

Best regards!

@eyusupov
Copy link
Contributor Author

image
image

@eyusupov eyusupov marked this pull request as ready for review April 22, 2024 17:36
@eyusupov
Copy link
Contributor Author

Updated

const char *ps_property_dim_names[] = {"empty_design", "empty", "now", "full", "full_design",
"empty_design", "empty", "now", "full", "full_design",
"min_design", "min", "now", "max", "max_design"};
const char *ps_property_dim_names[] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello,

I've tested your code, and it seems to be functional. However, upon reviewing it, I've come across some changes that I'm not entirely convinced about. Let me explain:

We're introducing two new rows here, along with a NULL at the end of each, but the "table" doesn't maintain a consistent number of columns for each line. This necessitates changes to the subsequent code.

I believe there's potential to simplify the changes you're proposing. My suggestion is to maintain the previous format of "ps_property_dim_names." Instead, consider creating a function within the loop that handles the necessary arguments for running the algorithm.

Following these adjustments, before entering the loop, you could execute calls for the lines that don't require five values, and then proceed with the loop for the remaining code.

I bring this up because I believe that introducing a "table" with varying patterns could potentially confuse some developers. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/collectors Everything related to data collection collectors/proc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants