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

Battery strategy 2 #62

Merged
merged 9 commits into from
May 16, 2022
Merged

Conversation

PastCoder
Copy link
Contributor

This time a little bigger change - actually it is not a change but an addition.

I propose an alternative strategy when to switch on/off the KOSTAL Smart Battery Management. The main information about the strategy is added in Readme.md and Readme_de.md. I've added there a section "Smart battery control".
The new strategy is added as option which can be selected in the adapter settings. The default strategy is the previous strategy.

(!) There is one change in the settings compare to the current github version: I changed the setting for the feed-in limitation from a percentage to an absolute value. It give a little more flexibility (e.g. if the actually panel efficiency is different from the nominal one).

I am using this strategy since a while now and it seems to work quite well.

(!) There is one thing which I did not manage: When you open the adapter settings, it shown always strategy 1 as current selection. I did nit figute out how to set the dropdown list to the previously select value when loading the settings.

When transferring the implementation from my experimental branch to this proposed one, I split the changes in three pull request:

  • Adjustment of the settings
  • Extract code into the function setSmartBatteryControl() to make the code a little more readable
  • Addition of the strategy 2

Test usual: Additional testing and feedback welcome :-) I am using strategy 2 so that this one wil lbe tested anyhow (at least with my setup).

Copy link
Contributor

@StrathCole StrathCole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution(s). Great work.

admin/index_m.html Show resolved Hide resolved
lib/plenticore.js Show resolved Hide resolved
@PastCoder
Copy link
Contributor Author

I have added code to migrate from old settings (either with max_feed_in_percentage or without) to the new settings in index_m.html and plenticore.js.
But as I don't know how I can remove existing values from the adapter config, I could not test the migration. I need to rely on a hint from you how to do that or to rely on your test/review.

Do you know how the make it work that the strategy dropdown updates to the actual setting when load the setting page?

@StrathCole
Copy link
Contributor

Do you know how the make it work that the strategy dropdown updates to the actual setting when load the setting page?

In index_m.html around line 47 there is a check for the input type. I think you need to loop through the corresponding option elements in the form and then add "selected" to that one matching the settings value.

@StrathCole
Copy link
Contributor

StrathCole commented May 15, 2022

But as I don't know how I can remove existing values from the adapter config, I could not test the migration. I need to rely on a hint from you how to do that or to rely on your test/review.

In index_m.html from line 127 (the save handler) you could just add something like obj['max_feed_in_percentage'] = null, I think.

@PastCoder
Copy link
Contributor Author

In index_m.html around line 47 there is a check for the input type. I think you need to loop through the corresponding option elements in the form and then add "selected" to that one matching the settings value.

I found a way how it works. Programming the logic is easy but as I am not a Javascript developer some of the stuff around is hard :-)

@PastCoder
Copy link
Contributor Author

add something like obj['max_feed_in_percentage'] = null

I've added this as proposed. Because I am not very familiar with Javascript I am not sure if the entry is now really gone or if it now jus has the value null. To be on the safe side, I added a check for null in getMaxFeedInPower(). By this I found a mistake there.

@PastCoder
Copy link
Contributor Author

Is in index_m.html the function fillInstances() still used? Or could this be removed to keep the code clean?

@StrathCole
Copy link
Contributor

Is in index_m.html the function fillInstances() still used? Or could this be removed to keep the code clean?

I think it was just for the weather forecast adapter selection and should no longer be used.

admin/index_m.html Outdated Show resolved Hide resolved
@StrathCole
Copy link
Contributor

I've added this as proposed. Because I am not very familiar with Javascript I am not sure if the entry is now really gone or if it now jus has the value null. To be on the safe side, I added a check for null in getMaxFeedInPower(). By this I found a mistake there.

Looks good to me how you solved it 👍

@PastCoder
Copy link
Contributor Author

I think it was just for the weather forecast adapter selection and should no longer be used.

I have removed it. Just as a proposal you can decide on.

@StrathCole StrathCole merged commit 49cf5d1 into pixcept:master May 16, 2022
@StrathCole
Copy link
Contributor

Thank you. I will issue a new release.

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 this pull request may close these issues.

None yet

2 participants