Skip to content
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.

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

AttributeError: 'PVOpt' object has no attribute 'intelligent' #300

Open
CraigCallender opened this issue Nov 18, 2024 · 20 comments · May be fixed by #301
Open

AttributeError: 'PVOpt' object has no attribute 'intelligent' #300

CraigCallender opened this issue Nov 18, 2024 · 20 comments · May be fixed by #301

Comments

@CraigCallender
Copy link
Contributor

Describe the bug

I can submit a PR with the fix for this as well.
When starting 3.18.2, I'm seeing the following error which stops pv_opt from starting up (note, I do not have IOG):

22:26:51 WARNING pv_opt: ------------------------------------------------------------
--
 
AttributeError: 'PVOpt' object has no attribute 'intelligent'
^^^^^^^^^^^^^^^^
elif self.intelligent:
File "/homeassistant/appdaemon/apps/pv_opt/pv_opt.py", line 1670, in optimise
^^^^^^^^^^^^^^
return f(*args, **kw)
File "/usr/lib/python3.11/site-packages/appdaemon/adbase.py", line 35, in f_app_lock
self.optimise()
File "/homeassistant/appdaemon/apps/pv_opt/pv_opt.py", line 1597, in optimise_state_change
^^^^^^^^^^^^^^
return f(*args, **kw)
File "/usr/lib/python3.11/site-packages/appdaemon/adbase.py", line 35, in f_app_lock
funcref(
File "/usr/lib/python3.11/site-packages/appdaemon/threading.py", line 1045, in worker
22:26:51 WARNING pv_opt: Traceback (most recent call last):
22:26:51 WARNING pv_opt: ------------------------------------------------------------
22:26:51 WARNING pv_opt: Worker Ags: {'id': 'ba5f098e733e4531a08899a539293a03', 'name': 'pv_opt', 'objectid': 'a4d586a88d324a0a9a316bd75580305d', 'type': 'state', 'function': <bound method PVOpt.optimise_state_change of <pv_opt.PVOpt object at 0xffff770a3290>>, 'attribute': 'state', 'entity': 'switch.pvopt_shape_consumption_profile', 'new_state': 'off', 'old_state': 'on', 'pin_app': True, 'pin_thread': 0, 'kwargs': {'__thread_id': 'thread-0'}}
22:26:51 WARNING pv_opt: Unexpected error in worker for App pv_opt:
22:26:51 WARNING pv_opt: ------------------------------------------------------------
22:26:51 WARNING pv_opt: ------------------------------------------------------------

To Reproduce
Steps to reproduce the behavior:

  1. Restart pv_opt.

Expected behavior
No error and pv_opt starts normally

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: homeassistant
  • Browser [e.g. chrome, safari]
  • Version [e.g. 22]

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

CraigCallender added a commit to CraigCallender/pv_opt_solis_cloud that referenced this issue Nov 18, 2024
@fboundy
Copy link
Owner

fboundy commented Nov 18, 2024 via email

@stevebuk1
Copy link
Collaborator

stevebuk1 commented Nov 18, 2024

Hmmmm, the setting of self.intelligent to False was first done in def load_contract.

The bug shows the optimiser must be being triggered to run before the contracts are loaded, which I would have thought would cause other issues if there are no prices loaded? Or does the optimiser now need to run before contracts are loaded for SolisCloud reasons? @CraigCallender Can you attach Pv_opt.log of a failing startup?

@fboundy
Copy link
Owner

fboundy commented Nov 18, 2024 via email

@stevebuk1
Copy link
Collaborator

Hi Francis,

Given the error log shows the optimiser run is being triggered by a state change rather than a scheduled run, then the PR is addressing the symptom and not the root cause. If the optimiser is running before contracts are loaded then its more likely to be a toggling of an entity that shouldn't be then triggering optimiser runs than it is setting self.intelligent to False earlier on in the startup process. A log should tell us.

@stevebuk1
Copy link
Collaborator

Hi @fboundy @CraigCallender as ultimately the code changes are mine I've had a go at reproducing this issue by forcing tariffs to be neither Agile or Intelligent but can't reproduce the problem using the Dev branch code. I'll have a go with the production release. @CraigCallender A full log of the issue would help me regenerate and solve.

@CraigCallender
Copy link
Contributor Author

Hey @stevebuk1 - So I've recently switched the the Octopus Cozy tariff and haven't been using pv_opt except for testing. The main reason is that I know have the same cheap price every cheap period, I don't see a reason to not charge my battery to full each session - whereas pv_opt seems to want to charge it the bare minimum until the next session.

I'll let it run overnight tonight through and see if I can recreate what I saw. This morning I've tried both restarting AppDaemon and restarting my HomeAssistant Green but I haven't seen a recreation of my initial report... so it could be an edge case or a race condition that caused it the first time (or something about the midnight change).

I'll report back in the morning of the 23rd....

@stevebuk1
Copy link
Collaborator

Hey @stevebuk1 - So I've recently switched the the Octopus Cozy tariff and haven't been using pv_opt except for testing. The main reason is that I know have the same cheap price every cheap period, I don't see a reason to not charge my battery to full each session - whereas pv_opt seems to want to charge it the bare minimum until the next session.

I'll let it run overnight tonight through and see if I can recreate what I saw. This morning I've tried both restarting AppDaemon and restarting my HomeAssistant Green but I haven't seen a recreation of my initial report... so it could be an edge case or a race condition that caused it the first time (or something about the midnight change).

I'll report back in the morning of the 23rd....

Hi Craig, thanks for the update. I've also tried to recreate by forcing an Eco7 tariff (so that the self.intelligent problamtic elsif is seen, which it won't be if running in Agile and the preceding if on self.agile is true) and found the same as you - its clean on a clean run but I did manage to recreate the problem if I toggled an entity half way through an optimiser run that was read early on in the process - in my case changing ev_charger from Zappi to None then caused a problem with a local variable ev_power. I think thats probably enough testing to rule out any more subtle root cause that would be masked if we made the change in your PR, and we can just do that. Theres probably a few more variable initialisations that I've done when needed rather than at initialisation so I will hunt them down and move them into initialise. I'll do this work on the Dev branch.

@fboundy
Copy link
Owner

fboundy commented Nov 22, 2024 via email

@stevebuk1
Copy link
Collaborator

stevebuk1 commented Nov 22, 2024

Yes - at this time of year on Cosy, Flux or E7 you might as well charge to full. Not the same in summer when there’s a chance Solar will do the job. Since Pv Opt starts from no force charge it stops when there’s no more benefit from more charging - the curve basically flattens off - but over a longer period there’s no cost to charging more because you are just borrowing from future charging if that makes sense

I've found that Pv_opt is most useful in spring and autumn when the amount of charge needed is variable from one night to the next. In summer it doesnt tend to trigger a charge plan and in winter it might as well fully charge.

For the tariffs with a fixed cheap rate I was contemplating adding a "soft minimum SOC" i.e. if on IOG, the charge plan is based having a target amount left in the battery at 11.30pm just before the cheap rate starts. I admit the idea comes from "best SOC keep" from Predbat. However I'm also aware that Pv_opt was designed from the outset to be simple to setup without stacks of parameters to configure (unlike predbat) - and I've tried to maintain that in the EV stuff by autosearching for entities etc. I guess the same can be achieved by adding a big consumption margin anyway? Thoughts on adding this target?

@CraigCallender
Copy link
Contributor Author

Not to derail too much from this issue (ha, maybe we should all exchange emails?), I think the feature is good and simpler than what I was thinking. What I'm noticing in winter with Cozy is that now that I have a heat pump (moved from gas to electric), in order to accurately predict my consumption today it probably needs to be based both on consumption history, but also weather forecasting. So in my mind something like you have with the EV charger of has_electric_heating with true or false would then use a weather integration to understand if your electric load will be higher, the same, or lower than your previous calculated consumption. An added complexity to this is consumption history will need to correlate past weather temperature as well.

And there's probably a way to have this work for people with air conditioning as well (we don't have it) as it would be a similar consumption shift if/when the weather gets too hot (though, less of an issue as hot weather tends to mean sunshine so the extra consumption can use solar).

@fboundy
Copy link
Owner

fboundy commented Nov 22, 2024 via email

@stevebuk1
Copy link
Collaborator

stevebuk1 commented Nov 22, 2024

So… Some of the reasons I wrote this in the first place were that I found Predbat: • Had an architecture that was heavily tied to GivEnergy products • Had an optimiser that did weird stuff with Agile • Had so many options it was confusing I think PVOpt still addresses the 2 but I’m a bit concerned about the 3rd. What it sounds like you need is a separate tool to forecast your consumption on a given day.

I almost went this way with the EV stuff, with a simple update to PV_opt to include a hook to populate the "car_slot" column in self.opt with an attribute from an external entity.

The new app/integration would have updated the aforementioned entity to provide the 1/2 hour slot 'hold' commands to PV_opt (be it a car charging plan from IOG, a car charge plan based on Agile rates etc) but as we are now discussing, also sort out the historic consumption data to remove the car charging data. I think the only thing that stopped me was that 6 months ago I hadn't written a single line of Python so my abilities weren't up to it.

I think the actual best answer would be to get with Trefor and the PredBat team and write a new tool that combines all our learning

You are probably not wrong......

@CraigCallender
Copy link
Contributor Author

@fboundy and @stevebuk1 - those are good insights and I agree, a separate consumption forecaster which emits to a entity would be better. I have a toddler... so who knows when I'll get around to that. :)

For now, pv_opt works really well for my summer use-case. Especially when I was on Agile. So please keep up all the hard work and I'll try and help out where I can. :)

@stevebuk1
Copy link
Collaborator

@fboundy and @stevebuk1 - those are good insights and I agree, a separate consumption forecaster which emits to a entity would be better. I have a toddler... so who knows when I'll get around to that. :)

I also have a toddler, who for the past few weeks has been determined to wake at 5.30am every morning (damn clocks change!). So my available hours is rapidly dwindling as I go to bed earlier to compensate ;-)

In all seriousness I probably can't continue to contribute at the rate I have been doing on the various issues tagged as enhancements. I'll need to wind down a bit once the multiple Zappi support and the Solarman integration work is complete and the EV stuff matures sufficiently to release to main.

@lazy-pete
Copy link

lazy-pete commented Nov 22, 2024 via email

@fboundy
Copy link
Owner

fboundy commented Nov 24, 2024 via email

@CraigCallender
Copy link
Contributor Author

So... I was able to recreate this and I think I know when it happens. Unfortunately, I didn't have it in debug logging mode when I did this. It looks like the two times I've gotten this issue has been on a HomeAssistant upgrade after it restarts. Regular restarts don't trigger this. Just on upgrade. I have no idea how to perform this to test without waiting for a Home Assistant update. I also can't remember if it's Home Assistant Core or not. (shrug). Any thoughts?

@stevebuk1
Copy link
Collaborator

Thanks Craig! I've got an update or two pending so I'll have a bash at recreating this.
Was your testing done with AppDaemon continuing to run, or was this done with the automation in place that restarts AppDaemon after an HA restart as per Section 13 of the documentation, script reproduced below?

alias: Restart AppDaemon on HA Restart
description: ""
trigger:
  - event: start
    platform: homeassistant
condition: []
action:
  - service: hassio.addon_stop
    data:
      addon: a0d7b954_appdaemon
  - delay:
      hours: 0
      minutes: 1
      seconds: 0
      milliseconds: 0
  - wait_template: >
      {{(states('sensor.solcast_pv_forecast_forecast_today')| float(-1)>0) and
      (states('sensor.solis_battery_soc')| float(-1)>0)}}
    continue_on_timeout: true
  - service: hassio.addon_start
    data:
      addon: a0d7b954_appdaemon
mode: single

@CraigCallender
Copy link
Contributor Author

Hey @stevebuk1 - I do have a similar piece of automation to restart AppDaemon, but I don't have those wait_template sections. I will update my automation... Thanks!

@stevebuk1
Copy link
Collaborator

stevebuk1 commented Nov 28, 2024

Ok, so I'll try it without the wait_template sections.

I've noticed the callbacks to allow optimise to run are setup before load_contract runs, so I think theres a possibility of triggering an optimise via a state change without any contract. If it does this, self.intelligent test is the first if/elsif it will come across in def optimise(self) hence the error. Your PR fixes the self.intelligent problem but I suspect there will be other problems encountered afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants