cli emeter year and month functions fail

[BuongiornoTexas]

The emeter year and month function fail. I'm not good with diffs, but the relevant code in method emeter of cli.py is:

   if year:
        click.echo(f"== For year {year.year} ==")
        emeter_status = await dev.get_emeter_monthly(year.year)
    elif month:
        click.echo(f"== For month {month.month} of {month.year} ==")
        emeter_status = await dev.get_emeter_daily(year=month.year, month=month.month)
    else:
        emeter_status = dev.emeter_realtime

    if isinstance(emeter_status, list):
        for plug in emeter_status:
            index = emeter_status.index(plug) + 1
            click.echo(f"Plug {index}: {plug}")
    else:
        click.echo("Current: %s A" % emeter_status["current"])
        click.echo("Voltage: %s V" % emeter_status["voltage"])
        click.echo("Power: %s W" % emeter_status["power"])
        click.echo("Total consumption: %s kWh" % emeter_status["total"])

        click.echo("Today: %s kWh" % dev.emeter_today)
        click.echo("This month: %s kWh" % dev.emeter_this_month)

This code correctly returns a dict of either yearly or monthly data as emeter_status, but this data is not handled correctly in the second if. Modifying the first if as follows should address the problem:

    if year:
        click.echo(f"== For year {year.year} ==")
        click.echo(await dev.get_emeter_monthly(year.year))
        return
    elif month:
        click.echo(f"== For month {month.month} of {month.year} ==")
        click.echo(await dev.get_emeter_daily(year=month.year, month=month.month))
        return
    else:
        emeter_status = dev.emeter_realtime

[kirichkov]

I don't get your code. You are completely discarding the emeter_status variable in the if and elif sections.

[BuongiornoTexas]

It displays the value data to console and returns, which seems appropriate. Otherwise, the rest of the method will fail when emeter_status contains a dict of values.

As far as I can see emeter_status isn't used anywhere else.

[kirichkov]

Your patch will also break (I think) the metering for the Smart strip which features multiple plugs, each with its own meter, so it won't be a bug fix, but a feature removal.

How about you tell us what device you're querying and some more information about the errors you're receiving? The version you're using and getting an error is also important - maybe this is already fixed in the latest version.

[BuongiornoTexas]

Your patch will also break (I think) the metering for the Smart strip which features multiple plugs, each with its own meter, so it won't be a bug fix, but a feature removal.

Ah! I didn't know about the SmartStrip. That said, I think the same problem occurs and has the same fix. See below.

How about you tell us what device you're querying and some more information about the errors you're receiving? The version you're using and getting an error is also important - maybe this is already fixed in the latest version.

I'm on the latest version, 0.4.0.dev1 and using an HS110.

The error occurs when I run either of the following commands:

kasa --host fruitbat emeter --year 2020
kasa --host fruitbat emeter --month 2020-8

Both fail with:

  File "d:\python envs\test\lib\site-packages\kasa\cli.py", line 338, in emeter
    click.echo("Current: %s A" % emeter_status["current"])
KeyError: 'current'

Which is not surprising, as the get_emeter_daily and get_emeter_monthly methods return dictionaries of the form Dict[int, float], where the ints are the day or month numbers and the floats are the period usage (so it can't have a "Current" key).

This applies for both the smartplug with emeter and the smartstrip. From checking the smartstrip code, it looks as though the smartstrip methods return the sum of the usage of the emeters on the strip (and not a list of usages from each individual emeter).

So my original fix should work for both a plain emeter and a smart strip.

If the smart strip is supposed to be reporting data for each emeter individually, then there need to be changes in both the smartstrip get_emeter_monthly/daily methods and in cli.py.

[kirichkov]

You're right - the CLI is referring to an old data structure I think. The code should return a number, representing the usage in kWh and not a dict - https://github.com/python-kasa/python-kasa/blob/master/kasa/smartdevice.py#L525.

I still think the better way to fix this is to remove these lines:

        click.echo("Current: %s A" % emeter_status["current"])
        click.echo("Voltage: %s V" % emeter_status["voltage"])
        click.echo("Power: %s W" % emeter_status["power"])

and change the next one to:

        click.echo("Total consumption: %s kWh" % emeter_status)

Can you open a PR (otherwise your contribution credit gets lost)?

[BuongiornoTexas]

Giorgi,

Can I check that you are suggesting returning only single number usage values from emeter calls? If this is what you mean, then people who use the cli (like me) would lose a lot of useful information. If this is not what you meant, could you clarify your explanation a bit more?

I think the cli with my (inelegant) fix delivers pretty well what was intended based on the data returned in emeter_status:

• A call without arguments such as 'kasa --host fruitbat emeter' provides current/instantaneous information of the form:

  == Emeter ==
  Current: 0.029 A
  Voltage: 243.106 V
  Power: 1.955 W
  Total consumption: 0.117 kWh
  Today: 0.02 kWh
  This month: 0.146 kWh

• A yearly data call ('kasa --host fruitbat emeter --year 2020`) summarises total kWh usage per month in the year. E.g. if I reset the meter in August 2020 and get readings in September, I'd see something like this for months 8 and 9:

  {8: 0.35; 9: 0.146}

  For a smart strip, each montly reading is the sum of the individual readings for each meter on the strip.

• A monthly data call ('kasa --host fruitbat emeter --month 2020-9'`) summarises total kWh usage per day in the month . E.g. if I reset the meter on September 5 August 2020 and get readings for September 2020, I'd see something like this:

  == For month 9 of 2020 ==
  {5: 0.02, 6: 0.024, 7: 0.028, 8: 0.026, 9: 0.028, 10: 0.02}

If I'm understanding your fix correctly, we would need to do further work to extract data from each emeter_status return value, and we would lose useful data along the way.

I'm happy to take a shot at the PR (a new thing to learn) once we agree on a solution.

Cheers,

Larry

[kirichkov]

Can I check that you are suggesting returning only single number usage values from emeter calls? If this is what you mean, then people who use the cli (like me) would lose a lot of useful information. If this is not what you meant, could you clarify your explanation a bit more?

That's what I meant, but now that you put your concerns forward, my proposed solution is not really a solution.

I think the cli with my (inelegant) fix delivers pretty well what was intended based on the data returned in emeter_status:

The issue is that while it fixes your problem, it will deliver a new problem for other users, that's why I don't think it's a proper fix.

I'd suggest the following - do a PR, without being afraid. I'll help the best I can along the way.

You'd need to find a fix by seeing what data is your plug returning. It looks like you are not getting a Dict but probably an integer/float value. Do you think you can do that? Hints: If you use an IDE you can use its debugger, if you're using a simple text editor the PDB module will help you to see what value you're getting.

Crash course on PDB:

At the top of the file you do:

import pdb

and then

    else:
        emeter_status = dev.emeter_realtime

   pdb.set_trace()

    if isinstance(emeter_status, list):
        for plug in emeter_status:

then you execute the CLI as you normally would, which will give you a prompt where you can expect the variable emeter_status. You can post the output here and we can decide on a solution, unless you figure it out yourself beforehand!

[BuongiornoTexas]

The issue is that while it fixes your problem, it will deliver a new problem for other users

After rechecking the code, I'm struggling to see how my change will be a problem. The master code is:

   if year:
        click.echo(f"== For year {year.year} ==")
        emeter_status = await dev.get_emeter_monthly(year.year)
    elif month:
        click.echo(f"== For month {month.month} of {month.year} ==")
        emeter_status = await dev.get_emeter_daily(year=month.year, month=month.month)
    else:
        emeter_status = dev.emeter_realtime

The get_emeter_daily and get_emeter_monthly methods return dicts mapping either day number or month number to device usage in the period (e.g. {8: 0.35; 9: 0.146}). This applies for all definitions of these methods (smartdevice.py and smartstrip.py). These dicts do not contain anything else, and consequently the pretty printing block fails because it is expecting a more complex data structure than these methods return! Given this, for these two clauses it should be perfectly safe to echo this data and return.

The emeter_status returned by the else clause is more complex (and completely different to the returns from get_emeter_daily/monthly methods). It presumably requires the more complex output processing block that follows. I'm not proposing to change this clause or the output processing block.

I'd suggest the following - do a PR, without being afraid. I'll help the best I can along the way.

Will do - I'll link to this issue.

[rytilahti]

This is fixed with #103.