Added New Operation In Mobile Device Action#4326
Merged
Maheshkale447 merged 4 commits intoOct 10, 2025
Merged
Conversation
Set Geo Location
Contributor
WalkthroughAdds a SetDeviceLocation action: introduces Latitude/Longitude/Altitude properties on ActMobileDevice, adds a collapsed UI panel with three UCValueExpression fields to ActMobileDeviceEditPage, and implements GenericAppiumDriver.SetDeviceLocation to apply GPS location via Appium. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as ActMobileDeviceEditPage
participant Act as ActMobileDevice
participant Driver as GenericAppiumDriver
participant Appium as AndroidDriver/IOSDriver
User->>UI: Select "SetDeviceLocation"
UI->>UI: Reveal xDeviceLoactioPnl (lat/long/alt)
User->>UI: Enter Latitude/Longitude/Altitude
UI->>Act: Bind values to properties
Note over Act,Driver: Execution
Act->>Driver: Execute(SetDeviceLocation)
Driver->>Driver: Validate & parse inputs (culture-invariant)
alt Valid inputs & supported driver
Driver->>Appium: Set location (lat,long,alt)
Appium-->>Driver: Success
Driver-->>Act: Complete
else Invalid inputs or unsupported driver
Driver-->>Act: Throw/return error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml(1 hunks)Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs(3 hunks)Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs(2 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (2)
Ginger/GingerCoreCommon/Actions/Act.cs (1)
AddOrUpdateInputParamValue(828-851)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
SetDeviceLocation(5492-5527)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (2)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (1)
ActMobileDevice(30-733)Ginger/GingerCoreCommon/Actions/Act.cs (1)
AddOrUpdateReturnParamActual(1308-1335)
Ginger/Ginger/Actions/ActionEditPages/ActMobileDeviceEditPage.xaml.cs (2)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (1)
ActMobileDevice(30-733)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (2)
ActMobileDevice(2077-2087)SetDeviceLocation(5492-5527)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs(2 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (2)
Ginger/GingerCoreCommon/Actions/Act.cs (1)
AddOrUpdateInputParamValue(828-851)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
SetDeviceLocation(5493-5556)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (2)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (1)
ActMobileDevice(30-733)Ginger/GingerCoreCommon/Actions/Act.cs (1)
AddOrUpdateReturnParamActual(1308-1335)
🔇 Additional comments (3)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (1)
620-622: Enum addition looks goodNew SetDeviceLocation action is well-named and documented.
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (2)
61-61: Import addition is appropriateUsing CultureInfo is correct for invariant numeric parsing.
1570-1573: Handler wiring looks correctSwitch case correctly invokes SetDeviceLocation(act).
…Feature/SetGeoLocationForMobileAction
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs(2 hunks)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (2)
Ginger/GingerCoreCommon/Actions/Act.cs (1)
AddOrUpdateInputParamValue(828-851)Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
SetDeviceLocation(5493-5564)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (2)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (1)
ActMobileDevice(30-733)Ginger/GingerCoreCommon/Actions/Act.cs (1)
AddOrUpdateReturnParamActual(1308-1335)
🔇 Additional comments (4)
Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs (2)
370-381: Altitude optional default looks goodEmpty default aligns with optional semantics and driver behavior (defaults to 0).
620-621: Enum addition LGTMSetDeviceLocation action added with clear description.
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (2)
1570-1572: Switch case wiring LGTMAction routes correctly to SetDeviceLocation(act).
5493-5564: SetDeviceLocation implementation looks solidNull checks, invariant parsing, range validation, optional altitude, and driver assignment are correct.
Comment on lines
+344
to
+355
| public ActInputValue Latitude | ||
| { | ||
| get | ||
| { | ||
| return GetOrCreateInputParam(nameof(Latitude),"0"); | ||
| } | ||
| set | ||
| { | ||
| AddOrUpdateInputParamValue(nameof(Latitude), value.ToString()); | ||
| OnPropertyChanged(nameof(Latitude)); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid defaulting Latitude to "0"; use empty to force explicit input
Default "0" makes a no-op action set location to (0,0). Prefer empty default so missing input fails early and doesn’t silently relocate the device.
Apply this diff:
- return GetOrCreateInputParam(nameof(Latitude),"0");
+ return GetOrCreateInputParam(nameof(Latitude), "");📝 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 ActInputValue Latitude | |
| { | |
| get | |
| { | |
| return GetOrCreateInputParam(nameof(Latitude),"0"); | |
| } | |
| set | |
| { | |
| AddOrUpdateInputParamValue(nameof(Latitude), value.ToString()); | |
| OnPropertyChanged(nameof(Latitude)); | |
| } | |
| } | |
| public ActInputValue Latitude | |
| { | |
| get | |
| { | |
| return GetOrCreateInputParam(nameof(Latitude), ""); | |
| } | |
| set | |
| { | |
| AddOrUpdateInputParamValue(nameof(Latitude), value.ToString()); | |
| OnPropertyChanged(nameof(Latitude)); | |
| } | |
| } |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs around lines 344
to 355, the Latitude getter currently defaults the input param to "0" which can
silently move the device to (0,0); change the default value passed to
GetOrCreateInputParam from "0" to an empty string ("") so the parameter is empty
by default and forces explicit input/validation upstream; keep the setter as-is
and ensure any callers/validation now treat an empty string as missing input.
Comment on lines
+357
to
+368
| public ActInputValue Longitude | ||
| { | ||
| get | ||
| { | ||
| return GetOrCreateInputParam(nameof(Longitude),"0"); | ||
| } | ||
| set | ||
| { | ||
| AddOrUpdateInputParamValue(nameof(Longitude), value.ToString()); | ||
| OnPropertyChanged(nameof(Longitude)); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid defaulting Longitude to "0"; use empty to force explicit input
Same rationale as Latitude: prevent unintended relocation to (0,0) when left blank.
Apply this diff:
- return GetOrCreateInputParam(nameof(Longitude),"0");
+ return GetOrCreateInputParam(nameof(Longitude), "");📝 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 ActInputValue Longitude | |
| { | |
| get | |
| { | |
| return GetOrCreateInputParam(nameof(Longitude),"0"); | |
| } | |
| set | |
| { | |
| AddOrUpdateInputParamValue(nameof(Longitude), value.ToString()); | |
| OnPropertyChanged(nameof(Longitude)); | |
| } | |
| } | |
| public ActInputValue Longitude | |
| { | |
| get | |
| { | |
| return GetOrCreateInputParam(nameof(Longitude), ""); | |
| } | |
| set | |
| { | |
| AddOrUpdateInputParamValue(nameof(Longitude), value.ToString()); | |
| OnPropertyChanged(nameof(Longitude)); | |
| } | |
| } |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/ActionsLib/UI/Mobile/ActMobileDevice.cs around lines 357
to 368, the Longitude property getter currently defaults to "0" which can
unintentionally relocate to (0,0); change the default value in
GetOrCreateInputParam from "0" to an empty string ("") so Longitude is blank
unless explicitly provided (mirror the Latitude behavior), leaving the setter
unchanged and ensuring OnPropertyChanged is still called.
Comment on lines
+5411
to
5413
| act.AddOrUpdateReturnParamActual("Latitude", location.Latitude.ToString()); | ||
| act.AddOrUpdateReturnParamActual("Longitude", location.Longitude.ToString()); | ||
| act.AddOrUpdateReturnParamActual("Altitude", location.Altitude.ToString()); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use invariant ToString for numeric return values
Avoid locale-specific commas/periods in outputs.
Apply this diff:
- act.AddOrUpdateReturnParamActual("Latitude", location.Latitude.ToString());
- act.AddOrUpdateReturnParamActual("Longitude", location.Longitude.ToString());
- act.AddOrUpdateReturnParamActual("Altitude", location.Altitude.ToString());
+ act.AddOrUpdateReturnParamActual("Latitude", location.Latitude.ToString(CultureInfo.InvariantCulture));
+ act.AddOrUpdateReturnParamActual("Longitude", location.Longitude.ToString(CultureInfo.InvariantCulture));
+ act.AddOrUpdateReturnParamActual("Altitude", location.Altitude.ToString(CultureInfo.InvariantCulture));📝 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
| act.AddOrUpdateReturnParamActual("Latitude", location.Latitude.ToString()); | |
| act.AddOrUpdateReturnParamActual("Longitude", location.Longitude.ToString()); | |
| act.AddOrUpdateReturnParamActual("Altitude", location.Altitude.ToString()); | |
| act.AddOrUpdateReturnParamActual("Latitude", location.Latitude.ToString(CultureInfo.InvariantCulture)); | |
| act.AddOrUpdateReturnParamActual("Longitude", location.Longitude.ToString(CultureInfo.InvariantCulture)); | |
| act.AddOrUpdateReturnParamActual("Altitude", location.Altitude.ToString(CultureInfo.InvariantCulture)); |
🤖 Prompt for AI Agents
In Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
around lines 5411 to 5413, the code calls ToString() on numeric location values
which can produce locale-specific formatting; change these calls to use
invariant culture (e.g.,
location.Latitude.ToString(CultureInfo.InvariantCulture) or
location.Latitude.ToString("G", CultureInfo.InvariantCulture)) for Latitude,
Longitude, and Altitude and add a using System.Globalization; directive if it’s
not already present so the outputs are culture-invariant.
Maheshkale447
approved these changes
Oct 10, 2025
Maheshkale447
enabled auto-merge
October 10, 2025 14:44
Maheshkale447
disabled auto-merge
October 10, 2025 16:42
Maheshkale447
merged commit Oct 10, 2025
7e4594d
into
Releases/Official-Release
16 of 17 checks passed
15 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Set Geo Location
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit