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

Mobile enhancments and changes #4094

Merged
merged 5 commits into from
Feb 11, 2025

Conversation

MeniKadosh1
Copy link
Contributor

@MeniKadosh1 MeniKadosh1 commented Feb 10, 2025

Thank you for your contribution.
Before submitting this PR, please make sure:

  • PR description and commit message should describe the changes done in this PR
  • Verify the PR is pointing to correct branch i.e. Release or Beta branch if the code fix is for specific release , else point it to master
  • Latest Code from master or specific release branch is merged to your branch
  • No unwanted\commented\junk code is included
  • No new warning upon build solution
  • Code Summary\Comments are added to my code which explains what my code is doing
  • Existing unit test cases are passed
  • New Unit tests are added for your development
  • Sanity Tests are successfully executed for New and Existing Functionality
  • Verify that changes are compatible with all relevant browsers and platforms.
  • After creating pull request there should not be any conflicts
  • Resolve all Codacy comments
  • Builds and checks are passed before PR is sent for review
  • Resolve code review comments
  • Update the Help Library document to match any feature changes

Summary by CodeRabbit

  • New Features

    • Introduced multi-touch support in the mobile device editing interface with a dedicated touch operations panel.
    • Added comprehensive device performance metrics reporting (CPU, memory, network, battery, and general info) for enhanced monitoring.
    • Improved mobile UI automation with updated element and locator strategies.
  • Bug Fixes

    • Corrected file transfer path handling for more reliable file management during mobile operations.

Copy link
Contributor

coderabbitai bot commented Feb 10, 2025

Walkthrough

The changes add multi-touch support to the mobile device editing features. A new grid component is introduced in the XAML page and its corresponding code-behind has been enhanced with methods to initialize and manage multi-touch operations. In addition, the mobile device action model now includes a new property and enum value to handle multi-touch events. A dedicated class to encapsulate touch operations has been added. The GenericAppiumDriver has been updated with methods for retrieving device performance metrics, and the MobilePlatform class has been adjusted to include additional UI element locators and actions.

Changes

File(s) Change Summary
Ginger/.../ActMobileDeviceEditPage.xaml Added xmlns:Ginger namespace declaration and a new <Ginger:ucGrid> component configured for multi-touch operations (collapsed by default).
Ginger/.../ActMobileDeviceEditPage.xaml.cs Introduced new methods (SetMultiTouchGridView, AddTouchOperation) and updated control binding and visibility logic to support multi-touch operations.
GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs Added a new private field and public property for mobile touch operations; updated the action enum with PerformMultiTouch.
GingerCoreNET/ActionsLib/UI/Mobile/MobileTouchOperation.cs New file implementing the MobileTouchOperation class and the eFingerOperationType enum to handle touch operation parameters and state management.
GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs Added several methods for retrieving device metrics (CPU, memory, network, battery, general info) and updated file path handling in file transfer methods.
GingerCoreNET/Run/Platforms/PlatformsInfo/MobilePlatform.cs Modified methods to add UI element actions and additional locator types (iOSPredicateString, iOSClassChain); updated learning locators list.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant EditPage as ActMobileDeviceEditPage
    participant Device as ActMobileDevice
    participant TouchOp as MobileTouchOperation

    User->>EditPage: Open mobile device edit page
    EditPage->>Device: Load mobile touch operations
    EditPage->>EditPage: Initialize multi-touch grid (SetMultiTouchGridView)
    User->>EditPage: Request to add a new touch operation
    EditPage->>TouchOp: Invoke AddTouchOperation()
    TouchOp-->>Device: Update MobileTouchOperations list
    Device-->>EditPage: Reflect updated operations
Loading

Possibly related PRs

Suggested reviewers

  • Maheshkale447

Poem

I'm a rabbit hopping with glee,
Multi-touch magic now runs free.
New grids and methods pave the way,
For actions that now cleverly play.
Hop along on code so bright,
With new metrics shining in the light.
🐇✨ Code rabbits unite!


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🔭 Outside diff range comments (2)
Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/MobilePlatform.cs (2)

104-108: 🛠️ Refactor suggestion

Remove commented-out code blocks.

These commented code blocks should be removed as they:

  • Add noise to the codebase
  • Make the code harder to maintain
  • Are preserved in version control if needed

Also applies to: 133-137


180-182: 🧹 Nitpick (assertive)

Add XML documentation and remove commented code.

Consider these improvements:

  1. Remove the commented-out base implementation call
  2. Add XML documentation to explain the method's purpose and return value

Example documentation:

+    /// <summary>
+    /// Returns a prioritized list of recommended element locators for mobile automation.
+    /// Each locator includes guidance on when and why to use it.
+    /// </summary>
+    /// <returns>An observable list of element locators with recommendations.</returns>
     public override ObservableList<ElementLocator> GetLearningLocators()
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 82d6d17 and 50e03d3.

📒 Files selected for processing (6)
  • Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml (3 hunks)
  • Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs (5 hunks)
  • Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (3 hunks)
  • Ginger/GingerCoreNET/ActionsLib/UI/Mobile/MobileTouchOperation.cs (1 hunks)
  • Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (14 hunks)
  • Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/MobilePlatform.cs (1 hunks)
🔇 Additional comments (25)
Ginger/GingerCoreNET/Run/Platforms/PlatformsInfo/MobilePlatform.cs (3)

119-132: LGTM! Changes align well with mobile platform requirements.

The modifications appropriately:

  • Add ClickXY for touch-based interactions
  • Remove mouse-specific operations
  • Ensure consistent click behavior
  • Remove window-specific elements

142-152: LGTM! Enhanced iOS support with appropriate locators.

The addition of iOS-specific locators (iOSPredicateString and iOSClassChain) improves the platform's capabilities while maintaining existing functionality.


180-194: LGTM! Well-structured locator recommendations.

The implementation provides clear guidance with prioritized locators and helpful descriptions for each strategy.

Ginger/GingerCoreNET/ActionsLib/UI/Mobile/MobileTouchOperation.cs (1)

118-128: Document the reason for empty ItemName implementation.

The ItemName property returns an empty string and has an empty setter without any explanation.

Please clarify:

  1. Why is an empty string returned?
  2. Why is the setter empty?
  3. Should this property be marked as [Obsolete] if it's not meant to be used?
Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml (1)

109-109: LGTM!

The new grid component is properly configured with appropriate properties and follows the existing UI layout patterns.

Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (1)

316-329: LGTM!

The changes are well-implemented:

  • The MobileTouchOperations property is properly serialized and follows the existing patterns.
  • The new enum value is appropriately placed and described.
  • Modern C# features are used (collection initializer syntax).

Also applies to: 511-512

Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (19)

40-40: New mobile UI actions import.
Bringing in Amdocs.Ginger.CoreNET.ActionsLib.UI.Mobile looks good and aligns with the new mobile touch operations.


719-722: GetXY case block.
Nicely retrieves the element's X/Y coordinates and sets them in act. Straightforward and consistent with the rest of the switch structure.


748-763: PullXandYofElement method.
Good approach returning false on failure and storing the coordinates when the element is found. Consider standardizing or localizing the error message to keep things consistent.


989-1023: GetAppPackage logic enhancements.
Smart approach to retrieve package/bundle ID from capabilities or from the current application if requested. Consider adding more logs or user-facing info when neither default nor current can be retrieved.


1287-1291: OpenDeeplink action.
Straightforward call to open a custom URL in the specified app. Might be worth adding logs or a fallback path if the deeplink fails.


1303-1307: Checking if the app is installed.
Sets a return parameter accordingly. Looks good. Possibly handle a default message when appPackage is empty.


1311-1315: Removing the app from device.
Follows the same pattern; no major concerns.


1319-1323: Querying the app state.
Adds the state to the action's return parameters. Straightforward and consistent with other device operations.


1418-1421: Enabling PerformMultiTouch action.
Smooth integration into the action switch. This broadens the device interaction capabilities.


1723-1724: Finalizing swipe action.
Completes the pointer-based gesture with a pointer-up event, then performs the action sequence. Implementation looks consistent.


2025-2037: GetCurrentPackage for both Android & iOS.
Be aware of potential string format issues on iOS if activeAppInfo doesn't contain a colon or isn't formatted as expected. Implement a fallback or error handling if needed.


3807-3811: GetDeviceCPUInfo method.
Returns a CPU info dictionary by invoking GetDeviceMetricsOverApiAsDict("cpuinfo"). Implementation is concise and clear.


3812-3816: GetDeviceMemoryInfo method.
Parallel to CPU usage retrieval. Straightforward with consistent naming.


3839-3843: GetDeviceBatteryInfo method.
Uses the same pattern to retrieve battery information as a dictionary. No issues.


3844-3855: GetDeviceGeneralInfo method.
Provides a dictionary of general device info, handling exceptions by returning empty. This is a sensible fallback.


3856-3872: GetDeviceMetricsOverApiAsDict: helpful fallback.
If the response is null or an error, returns an empty dictionary. This prevents unexpected crashes.


3874-3891: GetDeviceMetricOverAPI method.
Builds and sends a REST request to retrieve performance data from Appium. The approach is consistent with other new metric methods.


4165-4173: PushFileToDevice method.
Reading local file content and pushing to the device is well implemented. If the file doesn't exist locally or if the path is incorrect, the user will see an exception. Looks good overall.


4180-4187: PullFileFromDevice method.
Mirrors the push logic. Ensure that missing or invalid file paths from the device side are handled gracefully—some device platforms might raise an unhandled exception.

Comment on lines +52 to +53
private int? mMoveXcoordinatee;
[IsSerializedForLocalRepository]
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Fix typo in field name.

The field name has a typo: mMoveXcoordinatee (extra 'e' at the end).

-private int? mMoveXcoordinatee;
+private int? mMoveXcoordinate;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private int? mMoveXcoordinatee;
[IsSerializedForLocalRepository]
private int? mMoveXcoordinate;
[IsSerializedForLocalRepository]

Comment on lines +7 to +20
public class MobileTouchOperation : RepositoryItemBase
{

public enum eFingerOperationType
{
[EnumValueDescription("Finger Move")]
FingerMove,
[EnumValueDescription("Finger Down")]
FingerDown,
[EnumValueDescription("Finger Up")]
FingerUp,
Pause,
Cancel
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add XML documentation for the class and enum.

The class and enum lack XML documentation which would help other developers understand their purpose and usage.

Add documentation like this:

+/// <summary>
+/// Represents a mobile touch operation that can be performed on a device.
+/// </summary>
 public class MobileTouchOperation : RepositoryItemBase
 {
+    /// <summary>
+    /// Defines the types of finger operations that can be performed.
+    /// </summary>
     public enum eFingerOperationType
     {
         [EnumValueDescription("Finger Move")]
         FingerMove,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public class MobileTouchOperation : RepositoryItemBase
{
public enum eFingerOperationType
{
[EnumValueDescription("Finger Move")]
FingerMove,
[EnumValueDescription("Finger Down")]
FingerDown,
[EnumValueDescription("Finger Up")]
FingerUp,
Pause,
Cancel
}
/// <summary>
/// Represents a mobile touch operation that can be performed on a device.
/// </summary>
public class MobileTouchOperation : RepositoryItemBase
{
/// <summary>
/// Defines the types of finger operations that can be performed.
/// </summary>
public enum eFingerOperationType
{
[EnumValueDescription("Finger Move")]
FingerMove,
[EnumValueDescription("Finger Down")]
FingerDown,
[EnumValueDescription("Finger Up")]
FingerUp,
Pause,
Cancel
}

Comment on lines +382 to +392
private void AddTouchOperation(object sender, RoutedEventArgs e)
{
MobileTouchOperation tuchOperation = new MobileTouchOperation
{
OperationType = MobileTouchOperation.eFingerOperationType.FingerMove,
MoveXcoordinate = 0,
MoveYcoordinate = 0,
OperationDuration = 200
};
mAct.MobileTouchOperations.Add(tuchOperation);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Extract default values to constants.

The default values for coordinates and duration are hardcoded in the AddTouchOperation method.

Extract these values to named constants for better maintainability:

+private const int DEFAULT_COORDINATE = 0;
+private const int DEFAULT_DURATION = 200;
+
 private void AddTouchOperation(object sender, RoutedEventArgs e)
 {
     MobileTouchOperation tuchOperation = new MobileTouchOperation
     {
         OperationType = MobileTouchOperation.eFingerOperationType.FingerMove,
-        MoveXcoordinate = 0,
-        MoveYcoordinate = 0,
-        OperationDuration = 200
+        MoveXcoordinate = DEFAULT_COORDINATE,
+        MoveYcoordinate = DEFAULT_COORDINATE,
+        OperationDuration = DEFAULT_DURATION
     };
     mAct.MobileTouchOperations.Add(tuchOperation);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private void AddTouchOperation(object sender, RoutedEventArgs e)
{
MobileTouchOperation tuchOperation = new MobileTouchOperation
{
OperationType = MobileTouchOperation.eFingerOperationType.FingerMove,
MoveXcoordinate = 0,
MoveYcoordinate = 0,
OperationDuration = 200
};
mAct.MobileTouchOperations.Add(tuchOperation);
}
private const int DEFAULT_COORDINATE = 0;
private const int DEFAULT_DURATION = 200;
private void AddTouchOperation(object sender, RoutedEventArgs e)
{
MobileTouchOperation tuchOperation = new MobileTouchOperation
{
OperationType = MobileTouchOperation.eFingerOperationType.FingerMove,
MoveXcoordinate = DEFAULT_COORDINATE,
MoveYcoordinate = DEFAULT_COORDINATE,
OperationDuration = DEFAULT_DURATION
};
mAct.MobileTouchOperations.Add(tuchOperation);
}

Comment on lines +364 to +380
private void SetMultiTouchGridView()
{
GridViewDef view = new GridViewDef(GridViewDef.DefaultViewName)
{
GridColsView =
[
new GridColView() { Field = nameof(MobileTouchOperation.OperationType), Header = "Operation Type", WidthWeight = 25,StyleType = GridColView.eGridColStyleType.ComboBox, CellValuesList=GingerCore.General.GetEnumValuesForCombo(typeof(eFingerOperationType)) },
new GridColView() { Field = nameof(MobileTouchOperation.MoveXcoordinate), Header = "X Coordinate (Move Only)", WidthWeight = 25, StyleType = GridColView.eGridColStyleType.Text },
new GridColView() { Field = nameof(MobileTouchOperation.MoveYcoordinate), Header = "Y Coordinate (Move Only)", WidthWeight = 25, StyleType = GridColView.eGridColStyleType.Text },
new GridColView() { Field = nameof(MobileTouchOperation.OperationDuration), Header = "Duration Milliseconds (Move & Pause)", WidthWeight = 25, StyleType = GridColView.eGridColStyleType.Text },
]
};
xMultiTouchGrid.btnRefresh.Visibility = Visibility.Collapsed;
xMultiTouchGrid.btnEdit.Visibility = Visibility.Collapsed;
xMultiTouchGrid.SetAllColumnsDefaultView(view);
xMultiTouchGrid.InitViewItems();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation for coordinate and duration fields.

The grid columns for coordinates and duration lack input validation, which could lead to invalid values being entered.

Add validation by implementing a custom ValidationRule or by handling the CellEditEnding event:

 new GridColView() { Field = nameof(MobileTouchOperation.MoveXcoordinate), Header = "X Coordinate (Move Only)", WidthWeight = 25, StyleType = GridColView.eGridColStyleType.Text },
+// Add validation
+xMultiTouchGrid.CellEditEnding += (s, e) =>
+{
+    if (e.Column.Header.ToString().Contains("Coordinate"))
+    {
+        if (!int.TryParse(e.EditingElement.ToString(), out _))
+        {
+            e.Cancel = true;
+            // Show error message
+        }
+    }
+};

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +722 to +733
case ActUIElement.eElementAction.ClickXY:
if (string.IsNullOrEmpty(act.GetInputParamCalculatedValue(ActUIElement.Fields.XCoordinate)) || string.IsNullOrEmpty(act.GetInputParamCalculatedValue(ActUIElement.Fields.YCoordinate)))
{
act.AddOrUpdateReturnParamActual("X", e.Location.X.ToString());
act.AddOrUpdateReturnParamActual("Y", e.Location.Y.ToString());
//get X and Y from element
if (PullXandYofElement(act))
{
TapXY(Convert.ToInt32(act.GetReturnParam("X")), Convert.ToInt32(act.GetReturnParam("Y")));
}
}
else
{
act.Error = "Element not found.";
TapXY(Convert.ToInt32(act.GetInputParamCalculatedValue(ActUIElement.Fields.XCoordinate)), Convert.ToInt32(act.GetInputParamCalculatedValue(ActUIElement.Fields.YCoordinate)));
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

ClickXY case block.
Automatically falls back to fetching the element's coordinates if the X or Y parameters are not provided. This is helpful, but consider verifying if retrieved coordinates might go out of screen bounds or if the element is partially offscreen.

Comment on lines +1177 to +1181
appPackage = GetAppPackage(act);
if (!string.IsNullOrEmpty(appPackage))
{
Driver.TerminateApp(appPackage);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

CloseApp operation with TerminateApp.
Similar to the OpenApp flow. If the termination fails or if the app is not running, an exception might occur. Consider adding handling around that to standardize error reporting.

Comment on lines +1384 to +1414
appPackage = GetAppPackage(act);
if (!string.IsNullOrEmpty(appPackage))
{
case ActMobileDevice.ePerformanceTypes.Cpuinfo:
{
GetSpecificPerformanceData(act.ActionAppPackage.ValueForDriver, act.PerformanceTypes.ToString(), act);
break;
}
case ActMobileDevice.ePerformanceTypes.Memoryinfo:
{
GetSpecificPerformanceData(act.ActionAppPackage.ValueForDriver, act.PerformanceTypes.ToString(), act);
break;
}
case ActMobileDevice.ePerformanceTypes.Batteryinfo:
{
GetSpecificPerformanceData(act.ActionAppPackage.ValueForDriver, act.PerformanceTypes.ToString(), act);
break;
}
case ActMobileDevice.ePerformanceTypes.Networkinfo:
{
GetSpecificPerformanceData(act.ActionAppPackage.ValueForDriver, act.PerformanceTypes.ToString(), act);
break;
}
case ActMobileDevice.ePerformanceTypes.Diskinfo:
{
GetSpecificPerformanceData(act.ActionAppPackage.ValueForDriver, act.PerformanceTypes.ToString(), act);
break;
}
switch (act.PerformanceTypes)
{
case ActMobileDevice.ePerformanceTypes.Cpuinfo:
{
GetSpecificPerformanceData(appPackage, act.PerformanceTypes.ToString(), act);
break;
}
case ActMobileDevice.ePerformanceTypes.Memoryinfo:
{
GetSpecificPerformanceData(appPackage, act.PerformanceTypes.ToString(), act);
break;
}
case ActMobileDevice.ePerformanceTypes.Batteryinfo:
{
GetSpecificPerformanceData(appPackage, act.PerformanceTypes.ToString(), act);
break;
}
case ActMobileDevice.ePerformanceTypes.Networkinfo:
{
GetSpecificPerformanceData(appPackage, act.PerformanceTypes.ToString(), act);
break;
}
case ActMobileDevice.ePerformanceTypes.Diskinfo:
{
GetSpecificPerformanceData(appPackage, act.PerformanceTypes.ToString(), act);
break;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

GetSpecificPerformanceData block.
Handles multiple performance metrics by switching on PerformanceTypes. The code is somewhat repetitive per case. A default case or a consolidated approach could reduce duplication and handle unexpected values more gracefully.

Comment on lines +1727 to +1767
public void PerformMultiTouch(ObservableList<MobileTouchOperation> operations)
{
AppiumInteractions.PointerInputDevice finger = new AppiumInteractions.PointerInputDevice(PointerKind.Touch);
ActionBuilder actionBuilder = new ActionBuilder();
foreach (MobileTouchOperation operation in operations)
{
int xCoordinate = 0;
int yCoordinate = 0;
TimeSpan duration = TimeSpan.Zero;
if (operation.OperationDuration != null)
{
duration = TimeSpan.FromMilliseconds(int.Parse(operation.OperationDuration.ToString()));
}
if (operation.MoveXcoordinate != null)
{
xCoordinate = int.Parse(operation.MoveXcoordinate.ToString());
}
if (operation.MoveYcoordinate != null)
{
yCoordinate = int.Parse(operation.MoveYcoordinate.ToString());
}
switch (operation.OperationType)
{
//all operations
case MobileTouchOperation.eFingerOperationType.FingerMove:
actionBuilder.AddAction(finger.CreatePointerMove(CoordinateOrigin.Viewport, xCoordinate, yCoordinate, duration));
break;
case MobileTouchOperation.eFingerOperationType.FingerDown:
actionBuilder.AddAction(finger.CreatePointerDown(PointerButton.TouchContact));
break;
case MobileTouchOperation.eFingerOperationType.FingerUp:
actionBuilder.AddAction(finger.CreatePointerUp(PointerButton.TouchContact));
break;
case MobileTouchOperation.eFingerOperationType.Pause:
actionBuilder.AddAction(finger.CreatePause(duration));
break;
case MobileTouchOperation.eFingerOperationType.Cancel:
actionBuilder.AddAction(finger.CreatePointerCancel());
break;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

PerformMultiTouch method.
Iterates over each MobileTouchOperation to build a pointer-based gesture sequence. Consider adding defensive checks for invalid durations or coordinates and perhaps logging errors if any step fails.

Comment on lines +3817 to +3837
public async Task<string> GetDeviceNetworkInfo()
{
return GetDeviceMetricOverAPI("networkinfo").Result;
//string url = this.AppiumServer + "/session/" + Driver.SessionId + "/appium/getPerformanceData";
//string package = GetCurrentPackage();
//if (!string.IsNullOrEmpty(package))
//{
// string requestBody = "{\"packageName\": \"" + package + "\", \"dataType\": \"networkinfo\"}";
// restClient = new RestClient(url);
// string response = await SendRestRequestAndGetResponse(url, requestBody).ConfigureAwait(false);
// if (response.Contains("error"))
// {
// return null;
// }
// return response;
//}
//else
//{
// return null;
//}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

GetDeviceNetworkInfo: recommend trimming the commented-out code.
The main logic delegates to GetDeviceMetricOverAPI("networkinfo"). The large commented-out block might be confusing—consider removing or converting to doc references.

Comment on lines +3960 to +3968
else
{
return [];
Dictionary<string, string> dict2 = new Dictionary<string, string>
{
{ "Bundle ID", (string)((IOSDriver)Driver).ExecuteScript("mobile: currentApp") }
};
return dict2;
}

}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

GetDeviceActivityAndPackage: iOS fallback.
The else branch returns a dictionary with the iOS bundle info. Consider aligning key naming to the Android version for cross-platform consistency, e.g. "Activity" vs "Bundle ID."

@MeniKadosh1 MeniKadosh1 merged commit 071875a into Releases/Official-Release Feb 11, 2025
10 of 11 checks passed
@MeniKadosh1 MeniKadosh1 deleted the Mobile-Enhancments-and-Changes branch February 11, 2025 09:06
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.

1 participant