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

Feature Added Percentage memory in widget #2335

Merged
merged 16 commits into from
Mar 19, 2024
Merged

Feature Added Percentage memory in widget #2335

merged 16 commits into from
Mar 19, 2024

Conversation

Gobinath-B
Copy link
Contributor

@Gobinath-B Gobinath-B commented Mar 1, 2024

Summary of the pull request

This pull request adds a new feature to the memory widget, displaying the memory usage percentage. It includes changes to both the frontend (Adaptive Card template) and the backend (C# code) to incorporate this functionality.

References and relevant issues

#1986

Detailed description of the pull request / Additional comments

The changes in this pull request involve:

  • Added memUsage to SystemMemoryTemplate.json from MemoryStats.cs for memory usage percentage.
  • Adding a new field in the Adaptive Card template to display memory usage percentage.
  • Implementing logic in the backend to calculate and provide the memory usage percentage.
  • Ensuring proper formatting and presentation of the memory usage percentage in the widget.

Additional comments or explanations, if any, can be provided here.

Validation steps performed

  • Tested the feature locally to verify correct display of memory usage percentage.
  • Reviewed code changes to ensure adherence to coding standards and best practices.
  • Reviewed UI changes to ensure consistency with design guidelines.
  • Ensured that existing functionality is not adversely affected by the new feature.

PR checklist

Screenshot (229)

@Gobinath-B
Copy link
Contributor Author

@Gobinath-B please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@krschau
Copy link
Collaborator

krschau commented Mar 1, 2024

Hi @Gobinath-B, thanks for working on this! I think you missed adding some files to this pull request, I only see the json template changes. I'm going to convert this to a draft PR, but please convert it back when the code is ready for review and we'll take a look!

@krschau krschau marked this pull request as draft March 1, 2024 17:11
@Gobinath-B
Copy link
Contributor Author

hai @krschau , when i started working on this the calculation code for memory percentage was already writed but it was never used in adaptive card .So i used that LoadContentData in adaptive card json.

   private string FloatToPercentString(float value)
   {
       return ((int)(value * 100)).ToString(CultureInfo.InvariantCulture) + "%";
   }
  memoryData.Add("memUsage", FloatToPercentString(currentData.MemUsage));

this is the function already wrote in SystemMemoryWidget.cs

@Gobinath-B
Copy link
Contributor Author

@krschau hi did you check my last comment

@Gobinath-B Gobinath-B marked this pull request as ready for review March 8, 2024 11:25
@krschau
Copy link
Collaborator

krschau commented Mar 8, 2024

Hi @Gobinath-B, sorry for the delayed reply. I see what you mean that the code was always there, and you're exposing it. Nice catch!

This week we're in stabilization mode to prepare for the 0.12 release, so we're only checking in a specific set of PRs right now. After this coming Tuesday, the main branch will open back up and we'll be ready to take this change. It would be great if you could update your fork to resolve the merge conflict that's showing, as well as removing the json file that got checked in. Thanks!

@Gobinath-B
Copy link
Contributor Author

@krschau Got it thanks for the reply

@Gobinath-B
Copy link
Contributor Author

hai @krschau is that the stabilization sprints are over?

@Gobinath-B Gobinath-B closed this Mar 16, 2024
@Gobinath-B Gobinath-B reopened this Mar 16, 2024
@Gobinath-B Gobinath-B closed this Mar 16, 2024
@Gobinath-B Gobinath-B reopened this Mar 16, 2024
CppProperties.json Outdated Show resolved Hide resolved
@Gobinath-B
Copy link
Contributor Author

@krschau finished the requested changes

@Gobinath-B
Copy link
Contributor Author

@krschau I done the changes you mentioned above. ready to review

@krschau
Copy link
Collaborator

krschau commented Mar 19, 2024

I tested the widget and it never fills in a number for Utilization. I don't see anywhere in the code that a variable called Utilization is being set. You need to add a variable to memoryData with the Percentage data.

@Gobinath-B
Copy link
Contributor Author

Okay lemme check

@Gobinath-B
Copy link
Contributor Author

Gobinath-B commented Mar 19, 2024

I tested the widget and it never fills in a number for Utilization. I don't see anywhere in the code that a variable called Utilization is being set. You need to add a variable to memoryData with the Percentage data.

@krschau actually i go through the code and find out there is a variable for Percentage data named memUsage

private string FloatToPercentString(float value)
 {
     return ((int)(value * 100)).ToString(CultureInfo.InvariantCulture) + "%";
 }
 var memoryData = new JsonObject();
 var currentData = dataManager.GetMemoryStats();

 memoryData.Add("memUsage", FloatToPercentString(currentData.MemUsage));

So that i put that variable name ${memUsage} in SystemMemoryTemplate.json


    {
          "type": "ColumnSet",
          "columns": [
            {
              "type": "Column",
              "items": [
                {
                  "text": "%Memory_Widget_Template/MemoryUsage%",
                  "type": "TextBlock",
                  "size": "small",
                  "isSubtle": true,
                  "horizontalAlignment": "right"
                },
                {
                  "text": "${memUsage}",
                  "type": "TextBlock",
                  "size": "medium",
                  "horizontalAlignment": "right"
                }
              ]
            }
          ]
        }

Ready to review @krschau ,Have a good day.

@krschau krschau requested a review from guimafelipe March 19, 2024 21:37
@krschau krschau merged commit 4c7381c into microsoft:main Mar 19, 2024
4 checks passed
@KurroLopez
Copy link

Thanks for your work. New feature is coming!!!

@Gobinath-B
Copy link
Contributor Author

@KurroLopez Thank you so much :)

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.

Percentage memory in widget
4 participants