Skip to content

Disallow non-targeted device commands #982

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.

Sign up for GitHub

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

Merged
sdb9696 merged 3 commits into from
Jun 17, 2024
+32 −25
Merged

Disallow non-targeted device commands #982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f757d9
Disallow non-targeted device commands
rytilahti Jun 15, 2024
f7a6284
Fix tests
rytilahti Jun 15, 2024
c2f3243
Merge remote-tracking branch 'upstream/master' into fix/disallow_non_…
sdb9696 Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 27 additions & 20 deletions kasa/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ def wrapper(message=None, *args, **kwargs):
echo = _do_echo


def error(msg: str):
"""Print an error and exit."""
echo(f"[bold red]{msg}[/bold red]")
sys.exit(1)


TYPE_TO_CLASS = {
"plug": IotPlug,
"switch": IotWallSwitch,
Expand Down Expand Up @@ -367,6 +373,9 @@ def _nop_echo(*args, **kwargs):
credentials = None

if host is None:
if ctx.invoked_subcommand and ctx.invoked_subcommand != "discover":
error("Only discover is available without --host or --alias")

echo("No host name given, trying discovery..")
return await ctx.invoke(discover)

Expand Down Expand Up @@ -764,7 +773,7 @@ async def emeter(dev: Device, index: int, name: str, year, month, erase):
"""
if index is not None or name is not None:
if not dev.is_strip:
echo("Index and name are only for power strips!")
error("Index and name are only for power strips!")
return

if index is not None:
Expand All @@ -774,11 +783,11 @@ async def emeter(dev: Device, index: int, name: str, year, month, erase):

echo("[bold]== Emeter ==[/bold]")
if not dev.has_emeter:
echo("Device has no emeter")
error("Device has no emeter")
return

if (year or month or erase) and not isinstance(dev, IotDevice):
echo("Device has no historical statistics")
error("Device has no historical statistics")
return
else:
dev = cast(IotDevice, dev)
Expand Down Expand Up @@ -865,7 +874,7 @@ async def usage(dev: Device, year, month, erase):
async def brightness(dev: Device, brightness: int, transition: int):
"""Get or set brightness."""
if not (light := dev.modules.get(Module.Light)) or not light.is_dimmable:
echo("This device does not support brightness.")
error("This device does not support brightness.")
return

if brightness is None:
Expand All @@ -885,7 +894,7 @@ async def brightness(dev: Device, brightness: int, transition: int):
async def temperature(dev: Device, temperature: int, transition: int):
"""Get or set color temperature."""
if not (light := dev.modules.get(Module.Light)) or not light.is_variable_color_temp:
echo("Device does not support color temperature")
error("Device does not support color temperature")
return

if temperature is None:
Expand All @@ -911,7 +920,7 @@ async def temperature(dev: Device, temperature: int, transition: int):
async def effect(dev: Device, ctx, effect):
"""Set an effect."""
if not (light_effect := dev.modules.get(Module.LightEffect)):
echo("Device does not support effects")
error("Device does not support effects")
return
if effect is None:
echo(
Expand Down Expand Up @@ -939,7 +948,7 @@ async def effect(dev: Device, ctx, effect):
async def hsv(dev: Device, ctx, h, s, v, transition):
"""Get or set color in HSV."""
if not (light := dev.modules.get(Module.Light)) or not light.is_color:
echo("Device does not support colors")
error("Device does not support colors")
return

if h is None and s is None and v is None:
Expand All @@ -958,7 +967,7 @@ async def hsv(dev: Device, ctx, h, s, v, transition):
async def led(dev: Device, state):
"""Get or set (Plug's) led state."""
if not (led := dev.modules.get(Module.Led)):
echo("Device does not support led.")
error("Device does not support led.")
return
if state is not None:
echo(f"Turning led to {state}")
Expand Down Expand Up @@ -1014,7 +1023,7 @@ async def on(dev: Device, index: int, name: str, transition: int):
"""Turn the device on."""
if index is not None or name is not None:
if not dev.children:
echo("Index and name are only for devices with children.")
error("Index and name are only for devices with children.")
return

if index is not None:
Expand All @@ -1035,7 +1044,7 @@ async def off(dev: Device, index: int, name: str, transition: int):
"""Turn the device off."""
if index is not None or name is not None:
if not dev.children:
echo("Index and name are only for devices with children.")
error("Index and name are only for devices with children.")
return

if index is not None:
Expand All @@ -1056,7 +1065,7 @@ async def toggle(dev: Device, index: int, name: str, transition: int):
"""Toggle the device on/off."""
if index is not None or name is not None:
if not dev.children:
echo("Index and name are only for devices with children.")
error("Index and name are only for devices with children.")
return

if index is not None:
Expand Down Expand Up @@ -1096,7 +1105,7 @@ def _schedule_list(dev, type):
for rule in sched.rules:
print(rule)
else:
echo(f"No rules of type {type}")
error(f"No rules of type {type}")

return sched.rules

Expand All @@ -1112,7 +1121,7 @@ async def delete_rule(dev, id):
echo(f"Deleting rule id {id}")
return await schedule.delete_rule(rule_to_delete)
else:
echo(f"No rule with id {id} was found")
error(f"No rule with id {id} was found")


@cli.group(invoke_without_command=True)
Expand All @@ -1128,7 +1137,7 @@ async def presets(ctx):
def presets_list(dev: IotBulb):
"""List presets."""
if not dev.is_bulb or not isinstance(dev, IotBulb):
echo("Presets only supported on iot bulbs")
error("Presets only supported on iot bulbs")
return

for preset in dev.presets:
Expand All @@ -1150,7 +1159,7 @@ async def presets_modify(dev: IotBulb, index, brightness, hue, saturation, tempe
if preset.index == index:
break
else:
echo(f"No preset found for index {index}")
error(f"No preset found for index {index}")
return

if brightness is not None:
Expand All @@ -1175,7 +1184,7 @@ async def presets_modify(dev: IotBulb, index, brightness, hue, saturation, tempe
async def turn_on_behavior(dev: IotBulb, type, last, preset):
"""Modify bulb turn-on behavior."""
if not dev.is_bulb or not isinstance(dev, IotBulb):
echo("Presets only supported on iot bulbs")
error("Presets only supported on iot bulbs")
return
settings = await dev.get_turn_on_behavior()
echo(f"Current turn on behavior: {settings}")
Expand Down Expand Up @@ -1212,9 +1221,7 @@ async def turn_on_behavior(dev: IotBulb, type, last, preset):
async def update_credentials(dev, username, password):
"""Update device credentials for authenticated devices."""
if not isinstance(dev, SmartDevice):
raise NotImplementedError(
"Credentials can only be updated on authenticated devices."
)
error("Credentials can only be updated on authenticated devices.")

click.confirm("Do you really want to replace the existing credentials?", abort=True)

Expand Down Expand Up @@ -1271,7 +1278,7 @@ async def feature(dev: Device, child: str, name: str, value):
return

if name not in dev.features:
echo(f"No feature by name '{name}'")
error(f"No feature by name '{name}'")
return

feat = dev.features[name]
Expand Down
10 changes: 5 additions & 5 deletions kasa/tests/test_cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,12 +461,12 @@ async def test_led(dev: Device, runner: CliRunner):

async def test_json_output(dev: Device, mocker, runner):
"""Test that the json output produces correct output."""
mocker.patch("kasa.Discover.discover", return_value={"127.0.0.1": dev})
# These will mock the features to avoid accessing non-existing
mocker.patch("kasa.Discover.discover_single", return_value=dev)
# These will mock the features to avoid accessing non-existing ones
mocker.patch("kasa.device.Device.features", return_value={})
mocker.patch("kasa.iot.iotdevice.IotDevice.features", return_value={})

res = await runner.invoke(cli, ["--json", "state"], obj=dev)
res = await runner.invoke(cli, ["--host", "127.0.0.1", "--json", "state"], obj=dev)
assert res.exit_code == 0
assert json.loads(res.output) == dev.internal_state

Expand Down Expand Up @@ -789,7 +789,7 @@ async def test_errors(mocker, runner):
)
assert res.exit_code == 1
assert (
"Raised error: Managed to invoke callback without a context object of type 'Device' existing."
"Only discover is available without --host or --alias"
in res.output.replace("\n", "") # Remove newlines from rich formatting
)
assert isinstance(res.exception, SystemExit)
Expand Down Expand Up @@ -860,7 +860,7 @@ async def test_feature_missing(mocker, runner):
)
assert "No feature by name 'missing'" in res.output
assert "== Features ==" not in res.output
assert res.exit_code == 0
assert res.exit_code == 1


async def test_feature_set(mocker, runner):
Expand Down