Skip to content

Power and Thermal Specification for RPE - Front end (updated) #TODO #277

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

Merged
merged 12 commits into from
Dec 13, 2024

Conversation

shivaahir158
Copy link
Collaborator

@shivaahir158 shivaahir158 commented Nov 26, 2024

Motivate of the pull request

  • To address an existing issue. If so, please provide a link to the issue:
  • Breaking new feature. If so, please describe details in the description part.

Describe the technical details -

PR for Power and Thermal Specification block, the front end is ready, CSS has been taken care of, updated the GlobalStateProvider.js of these files, so it deals good with the backend, everything looks ok.

Intitial look of the feature:

image

When the user inputs are changed for Power and Thermal Specification:

image

Which part of the code base require a change

  • Frontend: powersummarytable.css, powersummarytable.js, globalstateprovider.test.js
  • Backend: api/device.py
  • Library: <Specify the library name, e.g. npm packages>
  • Plug-in: <Specify the plugin name, e.g. Webpack, jtest>
  • Documentation
  • Regression tests
  • Continuous Integration (CI) scripts

Impact of the pull request

  • Require a change on Quality of Results (QoR)
  • Break backward-compatibility. If so, please list who may be influenced.

Copy link

codecov bot commented Nov 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.40%. Comparing base (bb2f088) to head (632f4f4).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #277      +/-   ##
==========================================
+ Coverage   68.78%   70.40%   +1.62%     
==========================================
  Files         110       55      -55     
  Lines        9035     7026    -2009     
  Branches      402        0     -402     
==========================================
- Hits         6215     4947    -1268     
+ Misses       2820     2079     -741     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shivaahir158
Copy link
Collaborator Author

@chernyee please pull this branch and please test once, this should have the changes we discussed yesterday, we can discuss this once you are online.

function PowerSummaryTable({
title, data, total, percent,
title, data = [], total = 0, percent = 0, deviceId = 'MPW1',
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't hardcode this, it should be reading this from the selected device option.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh okay, right , this shall be done.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still not resolved, please look.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ravikiranchollangi , this is a delicate feature, it would be time consuming to do these changes, I shall do it, but can we push it for MPW1 for this release for time being? We have one device for now, so it works on itself.

The feature works now and I connected with @chernyee yesterday offline and he gave a thumbs up to the front end changes, just this hardcoding thing shall be taken care of in some time.

@ravikiranchollangi we can put this feature now is what I feel, please do let me know what you think and please merge it if everything looks ok.

Feature: tested and verified it works.

Copy link
Contributor

@ravikiranchollangi ravikiranchollangi left a comment

Choose a reason for hiding this comment

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

1 comment

Copy link
Contributor

@ravikiranchollangi ravikiranchollangi left a comment

Choose a reason for hiding this comment

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

1 more comment

@shivaahir158 shivaahir158 requested a review from chernyee December 3, 2024 22:03
@shivaahir158 shivaahir158 marked this pull request as ready for review December 5, 2024 14:03
@shivaahir158 shivaahir158 self-assigned this Dec 5, 2024
@shivaahir158 shivaahir158 changed the title Power and Thermal Specification for RPE - Front end (updated) Power and Thermal Specification for RPE - Front end (updated) #TODO Dec 13, 2024
@ravikiranchollangi ravikiranchollangi merged commit 2700373 into main Dec 13, 2024
1 of 6 checks passed
@ravikiranchollangi ravikiranchollangi deleted the thermal-power-spec-updated-api-endpoint branch December 13, 2024 18:20
@shivaahir158 shivaahir158 restored the thermal-power-spec-updated-api-endpoint branch December 13, 2024 18:30
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