feat(session): consolidate device actions into appium_mobile_device_control#259
feat(session): consolidate device actions into appium_mobile_device_control#259Mochxd wants to merge 2 commits intoappium:mainfrom
Conversation
a1243fe to
9c13fce
Compare
|
@KazuCocoa @saikrishna321 @SrinivasanTarget
Design preference
Naming
Behavior / review notes
i think we should not align on naming and action strings only but I’d prefer not folding device info into the same tool as lock/shake/notifications. |
39727c9 to
9c13fce
Compare
|
@Mochxd thanks for your valuable contribution and suggestion. I agree with you on the suggestion. The design principle should be: one user intent = one tool, regardless of how many Appium APIs it takes underneath.
"control device" and "query device" are fundamentally different intents. |
| PLATFORM, | ||
| } from '../../session-store.js'; | ||
| import { execute } from '../../command.js'; | ||
|
|
There was a problem hiding this comment.
seems like a giant. Can we do something like this?
async function handleLock(driver, seconds?: number): Promise { ... }
async function handleUnlock(driver): Promise { ... }
async function handleShake(driver): Promise { ... }
async function handleOpenNotifications(driver): Promise { ... }
There was a problem hiding this comment.
good catch as usual, done extracted handleLock, handleUnlock, handleShake, and handleOpenNotifications into separate functions, the execute block is now a thin switch/case dispatcher
| action: z | ||
| .enum(['lock', 'unlock', 'shake', 'open_notifications']) | ||
| .describe( | ||
| 'Action to perform: lock/unlock device, shake (iOS XCUITest only), or open notifications (Android only).' |
There was a problem hiding this comment.
.describe(
'Action to perform. ' +
'lock: lock the device (optional seconds for timed lock). ' +
'unlock: unlock the device. ' +
'shake: perform shake gesture (iOS XCUITest only). ' +
'open_notifications: open notifications panel (Android only).'
)
| server.addTool({ | ||
| name: 'appium_mobile_device_control', | ||
| description: | ||
| 'Control device-level actions in one tool. action=lock|unlock uses mobile: lock/unlock, action=shake performs iOS XCUITest shake, action=open_notifications opens Android notifications panel.', |
There was a problem hiding this comment.
Tool description could be more intent-oriented
There was a problem hiding this comment.
e.g. Control device behavior: lock/unlock the screen, shake the device, or open the notifications panel. Use the action parameter to choose what to do.
There was a problem hiding this comment.
done, i adopted your suggested intent-oriented description
| | `create_session` | Create a new mobile automation session for Android, iOS, or `general` capabilities (see 'general' mode above). If a remote Appium server is referenced, `create_session` forwards the final capabilities to that server via the WebDriver `newSession` API - include device selection (e.g., `appium:udid`) in `capabilities` when targeting a remote server. | | ||
| | `delete_session` | Delete the current mobile session and clean up resources | | ||
| | `appium_mobile_shake` | Shake gesture (`mobile: shake`) on **iOS Simulator only** (XCUITest). Not supported on Android or physical iOS devices. | | ||
| | `appium_mobile_device_control` | Device controls in one tool (`action`: `lock` \| `unlock` \| `shake` \| `open_notifications`). `shake` requires iOS XCUITest; `open_notifications` is Android only; `seconds` is optional for timed lock. | |
There was a problem hiding this comment.
drop "XCUITest" from the README
…ontrol Replace separate shake, notifications, and lock/unlock tools with one action-based tool (lock, unlock, shake, open_notifications) and platform-specific validation.
- Extract handler functions: handleLock, handleUnlock, handleShake, handleOpenNotifications for cleaner separation - Use switch/case dispatch in execute - Update action .describe() with per-action explanations - Make tool description intent-oriented per reviewer suggestion - Drop XCUITest reference from README description
9c13fce to
d7b22f7
Compare
Consolidates lock, unlock, shake, and open notifications into one intent-based tool:
appium_mobile_device_control.What changed
action: "lock" | "unlock" | "shake" | "open_notifications"secondsonly foraction="lock"(timed lock viamobile: lock)shake: iOS XCUITest sessions onlyopen_notifications: Android only (UiAutomator2openNotifications(), ormobile: openNotificationsfor remote sessions)src/tools/index.tsREADME.md,src/tools/README.md)Behavior
action="lock"→mobile: lock(optionalseconds)action="unlock"→mobile: unlockaction="shake"→ device shake (XCUITest)action="open_notifications"→ open notifications panel (Android)secondsis rejected whenactionis notlock.