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

Discrepancy in acceptable Charge Period time ranges in integration vs those in the FoxCloud App #367

Closed
Grimroper opened this issue Jul 15, 2023 · 10 comments · Fixed by #368

Comments

@Grimroper
Copy link

Hello

In the FoxCloud app when setting the time range to start and end your charging you can enter a matching time. For example you can enter a time of 00:00 to start and a time of 00:00 to end if you wanted to.

Previously the app wouldnt let you do that, it gave an error that said something like you cant have times overlapping. But now, probably when the app was updated to remove the "Enable force charge" option, you can now enter a matching start and end time.

The reason I want to set the matching times is because if you have "Enable Charge From Grid" on and also have a time range of say 00:30 to 01:30 the battery wont charge during that period but it also wont discharge for that hour either. Power will come from the grid or at least thats my experience of what happens.

However the integration still forces to you not overlap the times. If you try to enter the same time in each field it says

"Failed to call service foxess_modbus.update_charge_period. value must be at least 00:01:00 for dictionary value @ data['end']. Got None"

For the message above I entered a start and end time of 00:00. See the example Yaml below

I admit that being forced to set this by 1 minute is not a big deal but its probably a good idea to bring the integration inline with how the App behaves. Potentionally removing the "Enable force charge" option as well?

I have v1.6.2 of the Foxess_modbus integration installed

Configuration

service: foxess_modbus.update_charge_period
data:
  inverter: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
  enable_force_charge: false
  enable_charge_from_grid: false
  start: "00:00:00"
  end: "00:00:00"
  charge_period: "1"
@canton7
Copy link
Collaborator

canton7 commented Jul 15, 2023

but its probably a good idea to bring the integration inline with how the App behaves. Potentionally removing the "Enable force charge" option as well?

I disagree. I think disabling a charge period by setting both start and end to midnight is undiscoverable and unintuitive -- just look at the number of questions on the Facebook group going "How do I disable a charge period?". I'm not sure why the app changed its behaviour here, but having an explicit toggle is much clearer.

The reason I want to set the matching times is because if you have "Enable Charge From Grid" on and also have a time range of say 00:30 to 01:30 the battery wont charge during that period but it also wont discharge for that hour either. Power will come from the grid or at least thats my experience of what happens.

If you have matching start/end times (so the whole charge period is disabled) then the inverter will ignore the value of "Enable Charge From Grid". The logic that's implemented by the inverter and the new version of the foxess app is:

image

With this in mind, your sentence doesn't make sense: if the start/end times are the same then the whole period is disabled and has no effect.

If you try to enter the same time in each field it says

If you have enable_force_charge: false then you can just omit the start/end fields.

@canton7
Copy link
Collaborator

canton7 commented Jul 15, 2023

The validation here could be improved to ignore the start/end times completely when the period is disabled, but that's easier said than done with HA...

Actually that's untrue. Fix incoming

canton7 added a commit to canton7/foxess_modbus that referenced this issue Jul 15, 2023
…iod is disabled

We should let the user put what they want in here if the period is
disabled, as the times are ignored anyway.

Fixes: nathanmarlor#367
@canton7
Copy link
Collaborator

canton7 commented Jul 15, 2023

Right, we'll now let you specify matching start/end times if the period is disabled (as the times are ignored in this case anyway).

@Grimroper
Copy link
Author

Grimroper commented Jul 15, 2023

Hi, thanks for the comments above. I completely agree the way the app works makes no sense and a toggle is far clearer.

I also realised when re-reading my initial post there was a subtle but very important mistake in one sentance . If Id have written it properly in the first place it probably would have made more sense.

"The reason I want to set the matching times is because if you have "Enable Charge From Grid" off and also have a time range of say 00:30 to 01:30 the battery wont charge during that period but it also wont discharge for that hour either. Power will come from the grid or at least thats my experience of what happens."

The scenario is was thinking about was this. Imagine today I set Enable Charge From Grid to "On" and the charging times from 06:30 to 07:30. My battery would charge from the grid between 6:30am and 7:30am as you would expect. If the following day I didnt want to charge my battery, if I just turned off Enable Charge From Grid and dont remove those times the battery wouldnt charge during that hour but nor would it discharge.

Thats my reason for wanting to set the difference in start and end times to as small a period as possible, so the battery continues to discharge as confirmed here

@canton7
Copy link
Collaborator

canton7 commented Jul 15, 2023

The scenario is was thinking about was this. Imagine today I set Enable Charge From Grid to "On" and the charging times from 06:30 to 07:30. My battery would charge from the grid between 6:30am and 7:30am as you would expect. If the following day I didnt want to charge my battery, if I just turned off Enable Charge From Grid and dont remove those times the battery wouldnt charge during that hour but nor would it discharge.

So, in this integration, if you want to neither charge nor discharge, you would turn off "Enable charge from grid", but leave "Enable force charge" on? If you wanted to discharge as normal, then off "Enable force charge". No need to change the times at all.

I'm really not clear on why you want to set the difference between start and end times as small as possible? That doesn't seem connected to any of the other things you've mentioned.

@Grimroper
Copy link
Author

Grimroper commented Jul 15, 2023

So, in this integration, you would turn off "Enable charge from grid", but leave "Enable force charge" on?

No, I would turn them both off or on so they as the same as each other at all times.

The problem I trying to explain is when the start and end times are populated. If its set to start charging at 06:30 and end at 07:30am when the Enabled settings are on the battery does what you would expect, it charges. If you turn the Enable setting off those times arent cleared, they remain set. When you get to that time period the batterys neither charges or discharges. This is something you stated in this post here and matches what Im seeing.

Because of the way it behaves when there are times populated and Enable is off, Im wanting to clear/reset those times with a start of 00:00 and end of 00:00 so the battery continues to discharge.

@canton7
Copy link
Collaborator

canton7 commented Jul 15, 2023

If you turn the Enable setting off those times arent cleared, they remain set. When you get to that time period the batterys neither charges or discharges. This is something you stated in this post here and matches what Im seeing.

I'm not sure how else to explain this. Please try and understand.

When you disable "Enable force charge", that disables the charge window. The battery will neither charge, nor will it prevent discharge. (To do this, we send a start and end time of midnight to the inverter, and we remember your old start and end time in the integration.)

When you enable "Enable force charge", we will send the start and end time you set to the inverter.

In this integration (as in the old version of the app), you do not disable a charge period by setting the start and end time to midnight. That is an implementation detail which is hidden from the user. You disable a charge window by disabling "Enable force charge". When you do this, we send the right values to the inverter to disable the charge window.

When using this integration, forget the weird oddity with the app that you disable a charge window by setting the start and end time to midnight. This is not the UI that this integration exposes.

@Grimroper
Copy link
Author

When you disable "Enable force charge", that disables the charge window. The battery will neither charge, nor will it prevent discharge. (To do this, we send a start and end time of midnight to the inverter, and we remember your old start and end time in the integration.)

Ahhhhhhh, so you are already making this right in the back ground just by my disabling "Enable force charge" BUT using a superior method to the one used by the App. Im with you! Sorry for the misunderstanding.

I guess part of my confusion was both the time values still being shown in the integration but if they are ignored when disabling and set to midnight anyway thats exactly what I was trying to do.

Anyway, what you explained has answered and solved my original issue. Sorry it took me slow long to grasp how it works. Im also sorry Ive wasted your time is creating the change in #368 too!

@canton7
Copy link
Collaborator

canton7 commented Jul 15, 2023

I guess part of my confusion was both the time values still being shown in the integration but if they are ignored when disabling and set to midnight anyway thats exactly what I was trying to do.

Yeah there's not a lot we can do about that unfortunately. That's why I said you can just omit the start/end times if the charge period is disabled - no need to specify them.

The best solution is going to be https://github.com/nathanmarlor/foxess_modbus_charge_period_card (still in development, not quite ready for mass consumption).

Screenshot_20230715-191054

@Grimroper
Copy link
Author

That looks like a really promising addition to an already very impressive Integration

Thanks for all your comments above they are much appreciated

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