Add explicit Android TV handling for unsupported actions#4462
Merged
ravirk91 merged 3 commits intoMar 11, 2026
Conversation
Now, actions like PressMenuButton, OpenCamera, SimulatePhoto, and others set a clear error message when attempted on Android TV. Also updated SimulatePhotoOrBarcode to throw a specific exception for Android TV, improving error feedback and platform-specific handling.
Contributor
WalkthroughAdds Android TV-specific guards to GenericAppiumDriver: many mobile actions now return a standardized "Operation not supported for Android TV" error on Android TV devices, while Android and iOS flows remain unchanged; also introduces a centralized helper to produce the Android TV not-supported error. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
1518-1524:⚠️ Potential issue | 🟠 MajorAndroid TV pull-file currently no-ops instead of failing explicitly.
The new outer guard has no Android TV branch, so on Android TV this case just falls through to
breakwithout settingact.Error. That turns an unsupported action into a silent success.Proposed fix
case ActMobileDevice.eMobileDeviceAction.PullFileFromDevice: - if(DevicePlatformType == eDevicePlatformType.Android || DevicePlatformType == eDevicePlatformType.iOS) - - if (ValidateAndroidOnlyOperation(act, "Pull file to device")) - { - PullFileFromDevice(act.FilePathInput.ValueForDriver, act.FolderPathInput.ValueForDriver); - } + if (DevicePlatformType == eDevicePlatformType.Android || DevicePlatformType == eDevicePlatformType.iOS) + { + if (ValidateAndroidOnlyOperation(act, "Pull file to device")) + { + PullFileFromDevice(act.FilePathInput.ValueForDriver, act.FolderPathInput.ValueForDriver); + } + } + else if (DevicePlatformType == eDevicePlatformType.AndroidTv) + { + act.Error = "Operation not supported for Android TV."; + } break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs` around lines 1518 - 1524, The current outer platform guard using DevicePlatformType and eDevicePlatformType only covers Android and iOS, so on Android TV the Pull-file case falls through and returns success; update the guard to include the Android TV enum (eDevicePlatformType.AndroidTV) or add an explicit else that sets act.Error and a descriptive message; specifically modify the block around DevicePlatformType/eDevicePlatformType checks so that ValidateAndroidOnlyOperation(...) and PullFileFromDevice(act.FilePathInput.ValueForDriver, act.FolderPathInput.ValueForDriver) run for Android and AndroidTV, and if the platform is unsupported ensure act.Error is set (e.g., act.Error = "Pull file not supported on platform X") instead of silently breaking.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs`:
- Around line 1185-1188: Several branches set act.Error = "Operation not
supported for Android TV." without logging; add a small helper method (e.g.,
LogAndroidTvUnsupported(ActionResult act) or ReportAndroidTvUnsupported(string
context, ActionResult act)) and replace each direct assignment where
DevicePlatformType == eDevicePlatformType.AndroidTv (references in this file
around the existing checks and lines calling act.Error) with a call to that
helper which sets act.Error and calls Reporter.ToLog(eLogLevel.ERROR, message)
with the same standardized message (include context if available). Update all
occurrences listed (the current check and the other ranges mentioned) to use
this helper to centralize the message and satisfy the logging guideline.
- Line 6579: The Android TV branch in ToggleData() is unreachable because the
code checks "if (Driver is AndroidDriver)" before checking for TV; move or add
an explicit Android-TV detection check at the top of ToggleData() (before the
AndroidDriver type check) and throw the InvalidOperationException there for TV
sessions. Use the Appium driver capabilities or a helper (e.g., inspect
Driver.Capabilities["deviceType"] /
Driver.Capabilities.GetCapability("isAndroidTV") or an existing IsAndroidTV
helper) to detect TV sessions, and keep the AndroidDriver-specific toggle logic
after that check so the "Unsupported Action for Android TV" path can execute
when appropriate.
- Around line 1535-1548: The change to only check DevicePlatformType ==
eDevicePlatformType.Android causes iOS to silently skip GetDeviceLogs instead of
returning the previous unsupported-operation error; update the platform
branching in GenericAppiumDriver.cs so iOS is handled explicitly (either include
eDevicePlatformType.iOS in the Android branch condition if appropriate or add an
else if(DevicePlatformType == eDevicePlatformType.iOS) that sets act.Error to
the original unsupported message), keeping the AndroidTv branch intact and still
calling GetDeviceLogs via ValidateAndroidOnlyOperation for Android using the
existing GetDeviceLogs, ValidateAndroidOnlyOperation, and
act.AddOrUpdateReturnParamActual/Act.AddArtifactToAction symbols.
---
Outside diff comments:
In
`@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs`:
- Around line 1518-1524: The current outer platform guard using
DevicePlatformType and eDevicePlatformType only covers Android and iOS, so on
Android TV the Pull-file case falls through and returns success; update the
guard to include the Android TV enum (eDevicePlatformType.AndroidTV) or add an
explicit else that sets act.Error and a descriptive message; specifically modify
the block around DevicePlatformType/eDevicePlatformType checks so that
ValidateAndroidOnlyOperation(...) and
PullFileFromDevice(act.FilePathInput.ValueForDriver,
act.FolderPathInput.ValueForDriver) run for Android and AndroidTV, and if the
platform is unsupported ensure act.Error is set (e.g., act.Error = "Pull file
not supported on platform X") instead of silently breaking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 05ace5b8-184f-4bdb-9646-9af6e9e7aa09
📒 Files selected for processing (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
Comment on lines
+1185
to
+1188
| else if (DevicePlatformType == eDevicePlatformType.AndroidTv) | ||
| { | ||
| act.Error = "Operation not supported for Android TV."; | ||
| } |
Contributor
There was a problem hiding this comment.
Centralize and log the new Android TV unsupported-action failures.
These new branches all duplicate the same error assignment and none of them log the failure. A small helper would keep the message consistent and satisfy the repo logging rule.
As per coding guidelines, **/*.cs: Use Reporter.ToLog(eLogLevel.ERROR, message) for logging errors.
Also applies to: 1200-1203, 1223-1226, 1238-1241, 1333-1335, 1349-1351, 1384-1386, 1396-1398, 1423-1425, 1479-1481, 1511-1513, 1544-1546
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs`
around lines 1185 - 1188, Several branches set act.Error = "Operation not
supported for Android TV." without logging; add a small helper method (e.g.,
LogAndroidTvUnsupported(ActionResult act) or ReportAndroidTvUnsupported(string
context, ActionResult act)) and replace each direct assignment where
DevicePlatformType == eDevicePlatformType.AndroidTv (references in this file
around the existing checks and lines calling act.Error) with a call to that
helper which sets act.Error and calls Reporter.ToLog(eLogLevel.ERROR, message)
with the same standardized message (include context if available). Update all
occurrences listed (the current check and the other ranges mentioned) to use
this helper to centralize the message and satisfy the logging guideline.
Comment on lines
+1535
to
1548
| if (DevicePlatformType == eDevicePlatformType.Android) | ||
| { | ||
| string deviceLogsPath = GetDeviceLogs(act.FolderPathInput.ValueForDriver); | ||
| act.AddOrUpdateReturnParamActual("DeviceLogFilePath", deviceLogsPath); | ||
| Act.AddArtifactToAction(Path.GetFileName(deviceLogsPath), act, deviceLogsPath); | ||
| if (ValidateAndroidOnlyOperation(act, "Get Device Logs")) | ||
| { | ||
| string deviceLogsPath = GetDeviceLogs(act.FolderPathInput.ValueForDriver); | ||
| act.AddOrUpdateReturnParamActual("DeviceLogFilePath", deviceLogsPath); | ||
| Act.AddArtifactToAction(Path.GetFileName(deviceLogsPath), act, deviceLogsPath); | ||
| } | ||
| } | ||
| else if(DevicePlatformType == eDevicePlatformType.AndroidTv) | ||
| { | ||
| act.Error = "Operation not supported for Android TV."; | ||
| } | ||
| break; |
Contributor
There was a problem hiding this comment.
GetDeviceLogs regresses iOS into a silent pass.
By narrowing the first branch to Android only, iOS no longer gets the old unsupported-operation error and just exits the case without doing anything. This changes existing behavior outside the Android TV scope.
Proposed fix
case ActMobileDevice.eMobileDeviceAction.GetDeviceLogs:
if (DevicePlatformType == eDevicePlatformType.Android)
{
if (ValidateAndroidOnlyOperation(act, "Get Device Logs"))
{
string deviceLogsPath = GetDeviceLogs(act.FolderPathInput.ValueForDriver);
act.AddOrUpdateReturnParamActual("DeviceLogFilePath", deviceLogsPath);
Act.AddArtifactToAction(Path.GetFileName(deviceLogsPath), act, deviceLogsPath);
}
}
- else if(DevicePlatformType == eDevicePlatformType.AndroidTv)
+ else if (DevicePlatformType == eDevicePlatformType.AndroidTv)
{
act.Error = "Operation not supported for Android TV.";
}
+ else
+ {
+ act.Error = "Get Device Logs not supported for this mobile OS or application type.";
+ }
break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs`
around lines 1535 - 1548, The change to only check DevicePlatformType ==
eDevicePlatformType.Android causes iOS to silently skip GetDeviceLogs instead of
returning the previous unsupported-operation error; update the platform
branching in GenericAppiumDriver.cs so iOS is handled explicitly (either include
eDevicePlatformType.iOS in the Android branch condition if appropriate or add an
else if(DevicePlatformType == eDevicePlatformType.iOS) that sets act.Error to
the original unsupported message), keeping the AndroidTv branch intact and still
calling GetDeviceLogs via ValidateAndroidOnlyOperation for Android using the
existing GetDeviceLogs, ValidateAndroidOnlyOperation, and
act.AddOrUpdateReturnParamActual/Act.AddArtifactToAction symbols.
ravirk91
reviewed
Mar 11, 2026
|
|
||
| case ActMobileDevice.eMobileDeviceAction.GetDeviceLogs: | ||
| if (ValidateAndroidOnlyOperation(act, "Get Device Logs")) | ||
| if (DevicePlatformType == eDevicePlatformType.Android) |
Contributor
Author
There was a problem hiding this comment.
its already for android not for iOS
if (ValidateAndroidOnlyOperation(act, "Get Device Logs"))
also in the existing (ValidateAndroidOnlyOperation) validation is for only Android
private bool ValidateAndroidOnlyOperation(ActMobileDevice act, string operationName)
{
if (DevicePlatformType != eDevicePlatformType.Android)
{
act.Error = $"{operationName} not supported for this mobile OS or application type.";
return false;
}
return true;
}
so no need for iOS
ravirk91
reviewed
Mar 11, 2026
| else | ||
| { | ||
| throw new InvalidOperationException("Unsupported driver type"); | ||
| throw new InvalidOperationException("Unsupported Action for Android TV"); |
Collaborator
There was a problem hiding this comment.
pls also handle for ios and android mobile
Contributor
Author
There was a problem hiding this comment.
for ToggleData()
there is existing function in that for iOS its throwing error
public void ToggleData() //not working, no SIM to the device
{
if (Driver is AndroidDriver)
{
ConnectionType currentConnection = ((AndroidDriver)Driver).ConnectionType;
if (currentConnection.HasFlag(ConnectionType.DataOnly))
{
((AndroidDriver)Driver).ConnectionType = ConnectionType.None; // disable
}
else
{
((AndroidDriver)Driver).ConnectionType = ConnectionType.DataOnly; //enable
}
}
else if (Driver is IOSDriver)
{
throw new NotSupportedException("Toggling data services is not directly supported on iOS");
}
else
{
throw new InvalidOperationException("Unsupported Action for Android TV");
}
}
so need for iOS
for other actions I have added both devices and for android TV only it will throw exception error
Centralize error handling for unsupported Android TV actions into a single method, reducing code duplication. Update ToggleData to use DevicePlatformType checks instead of driver type, improving clarity and maintainability.
ravirk91
approved these changes
Mar 11, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (1)
1518-1524:⚠️ Potential issue | 🟠 Major
PullFileFromDevicestill silently no-ops on Android TV.Line 1518 starts an outer
ifwithout braces, so Android TV skips the inner validation and hitsbreakwithout any explicit unsupported-action error. That breaks the new Android TV handling this PR is trying to standardize.Proposed fix
case ActMobileDevice.eMobileDeviceAction.PullFileFromDevice: - if(DevicePlatformType == eDevicePlatformType.Android || DevicePlatformType == eDevicePlatformType.iOS) - - if (ValidateAndroidOnlyOperation(act, "Pull file to device")) - { - PullFileFromDevice(act.FilePathInput.ValueForDriver, act.FolderPathInput.ValueForDriver); - } + if (DevicePlatformType == eDevicePlatformType.Android || DevicePlatformType == eDevicePlatformType.iOS) + { + if (ValidateAndroidOnlyOperation(act, "Pull file from device")) + { + PullFileFromDevice(act.FilePathInput.ValueForDriver, act.FolderPathInput.ValueForDriver); + } + } + else if (DevicePlatformType == eDevicePlatformType.AndroidTv) + { + NotSupportedErrorForAndroidTv(act); + } break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs` around lines 1518 - 1524, The outer platform check around the PullFileFromDevice call lacks braces so Android TV falls through to the break without hitting validation; fix by adding braces to the outer if (DevicePlatformType == eDevicePlatformType.Android || DevicePlatformType == eDevicePlatformType.iOS) to encompass the inner if (ValidateAndroidOnlyOperation(...)) { PullFileFromDevice(...) ; } and ensure an else path for other platforms (e.g., Android TV) calls ValidateAndroidOnlyOperation(act, "Pull file to device") or otherwise raises the unsupported-action handling so the operation no-ops with an explicit error instead of silently breaking; reference DevicePlatformType, eDevicePlatformType.Android, eDevicePlatformType.iOS, ValidateAndroidOnlyOperation, and PullFileFromDevice.
♻️ Duplicate comments (2)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs (2)
1668-1671:⚠️ Potential issue | 🟡 MinorLog the centralized Android TV unsupported-action helper.
This helper now owns the unsupported-TV path for many actions, but it still sets
act.Errorwithout emitting the required error log.Proposed fix
private static void NotSupportedErrorForAndroidTv(ActMobileDevice act) { - act.Error = "Operation not supported for Android TV."; + const string message = "Operation not supported for Android TV."; + Reporter.ToLog(eLogLevel.ERROR, message); + act.Error = message; }As per coding guidelines,
**/*.cs: UseReporter.ToLog(eLogLevel.ERROR, message)for logging errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs` around lines 1668 - 1671, The helper NotSupportedErrorForAndroidTv currently only sets act.Error and must also emit a standardized error log; update NotSupportedErrorForAndroidTv(ActMobileDevice act) to call Reporter.ToLog(eLogLevel.ERROR, "<clear message>") with the same descriptive message ("Operation not supported for Android TV.") when setting act.Error so the unsupported-TV path is logged per guidelines.
1535-1548:⚠️ Potential issue | 🟠 Major
GetDeviceLogsregresses iOS into a silent pass.Line 1535 now limits the first branch to
Android, so iOS never reachesValidateAndroidOnlyOperation()anymore and exits this case without settingact.Error.Proposed fix
case ActMobileDevice.eMobileDeviceAction.GetDeviceLogs: if (DevicePlatformType == eDevicePlatformType.Android) { if (ValidateAndroidOnlyOperation(act, "Get Device Logs")) { string deviceLogsPath = GetDeviceLogs(act.FolderPathInput.ValueForDriver); act.AddOrUpdateReturnParamActual("DeviceLogFilePath", deviceLogsPath); Act.AddArtifactToAction(Path.GetFileName(deviceLogsPath), act, deviceLogsPath); } } - else if(DevicePlatformType == eDevicePlatformType.AndroidTv) + else if (DevicePlatformType == eDevicePlatformType.AndroidTv) { NotSupportedErrorForAndroidTv(act); } + else + { + act.Error = "Get Device Logs not supported for this mobile OS or application type."; + } break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs` around lines 1535 - 1548, The change restricts the first branch to only eDevicePlatformType.Android, causing iOS to fall through silently; restore explicit handling so iOS triggers the same validation/error behavior. Modify the branching around DevicePlatformType so that iOS either enters the same block that calls ValidateAndroidOnlyOperation/GetDeviceLogs (e.g. DevicePlatformType == eDevicePlatformType.Android || DevicePlatformType == eDevicePlatformType.iOS) or add an explicit else if for eDevicePlatformType.iOS that calls ValidateAndroidOnlyOperation(act, "Get Device Logs") and sets act.Error (and avoids calling GetDeviceLogs) so unsupported platforms do not silently pass; reference ValidateAndroidOnlyOperation, GetDeviceLogs, act.Error and Act.AddArtifactToAction when making the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs`:
- Around line 1518-1524: The outer platform check around the PullFileFromDevice
call lacks braces so Android TV falls through to the break without hitting
validation; fix by adding braces to the outer if (DevicePlatformType ==
eDevicePlatformType.Android || DevicePlatformType == eDevicePlatformType.iOS) to
encompass the inner if (ValidateAndroidOnlyOperation(...)) {
PullFileFromDevice(...) ; } and ensure an else path for other platforms (e.g.,
Android TV) calls ValidateAndroidOnlyOperation(act, "Pull file to device") or
otherwise raises the unsupported-action handling so the operation no-ops with an
explicit error instead of silently breaking; reference DevicePlatformType,
eDevicePlatformType.Android, eDevicePlatformType.iOS,
ValidateAndroidOnlyOperation, and PullFileFromDevice.
---
Duplicate comments:
In
`@Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs`:
- Around line 1668-1671: The helper NotSupportedErrorForAndroidTv currently only
sets act.Error and must also emit a standardized error log; update
NotSupportedErrorForAndroidTv(ActMobileDevice act) to call
Reporter.ToLog(eLogLevel.ERROR, "<clear message>") with the same descriptive
message ("Operation not supported for Android TV.") when setting act.Error so
the unsupported-TV path is logged per guidelines.
- Around line 1535-1548: The change restricts the first branch to only
eDevicePlatformType.Android, causing iOS to fall through silently; restore
explicit handling so iOS triggers the same validation/error behavior. Modify the
branching around DevicePlatformType so that iOS either enters the same block
that calls ValidateAndroidOnlyOperation/GetDeviceLogs (e.g. DevicePlatformType
== eDevicePlatformType.Android || DevicePlatformType == eDevicePlatformType.iOS)
or add an explicit else if for eDevicePlatformType.iOS that calls
ValidateAndroidOnlyOperation(act, "Get Device Logs") and sets act.Error (and
avoids calling GetDeviceLogs) so unsupported platforms do not silently pass;
reference ValidateAndroidOnlyOperation, GetDeviceLogs, act.Error and
Act.AddArtifactToAction when making the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e94b4e42-09c0-4f5f-9a61-5fb886bfb11c
📒 Files selected for processing (1)
Ginger/GingerCoreNET/Drivers/CoreDrivers/Mobile/Appium/GenericAppiumDriver.cs
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.
Now, actions like PressMenuButton, OpenCamera, SimulatePhoto, and others set a clear error message when attempted on Android TV. Also updated SimulatePhotoOrBarcode to throw a specific exception for Android TV, improving error feedback and platform-specific handling.
Description
Type of Change
Checklist
[IsSerializedForLocalRepository]where neededReporter.ToLog()patternSummary by CodeRabbit