Open almighty059 opened 10 months ago
Hey @almighty059, thanks for the feedback! Just coming back from a family emergency so I haven't had too much time recently for recreational coding. Additionally, I just switched computers, and I haven't had a chance to move all my development tools off my old rig yet. That said, I figured I'd pop in though and respond to what I can so you don't think I left you unread. :)
Plugged In or Unplugged: These status code are governed by whats available for the device class assigned to the component, and is part of HA Core. Looking at the documentation for Binary Sensor Entities, the device classes all report On
and Off
. Off the top of my head I'm not aware of how one would customize the status codes, but I agree that'd be much clearer to report Plugged
and Unplugged
instead. I'll look into if that's something that can be customized.
Auto mode variable name inconsistency: I'll look into this.
Readme inconsistency: Good catch.
Speed vs Level: Most of the entities ids reflect the data model they're referencing in order to tightly couple them to the data. In this case, the ACInfinity API fields in the json for those entities are onSpeed
and offSpeed
. I do agree though that level is a much better term for those entities. While I wouldn't want to change the key or variable names of the entities to keep that tight coupling to the data model (as well as avoid a breaking change for those that have already installed the integration), I think that changing the default label to level
is a good idea. That'd result in default HA entity ids like you described for new entities (old entities already registered with HA would need to be changed manually by the user).
Polling Delay: I might create a separate bug report for this one to look into this deeper.
_minutes_to_on and _minutes_to_off: This is a code clue to help inform other developers that the variable is configured in minutes, not second or hours. A duration of 3 hours would be stored as a value of 180
in this variable. This applies to cycle time as well.
Toggle Switches: Hmmm yeah. Its also inconsistent with the other enabled toggles. I must have got the wrong device class for those entities. I'll take a look.
Love the dashboard. Yeah I state in the readme, I was working on a lovelace card that would show and hide entities based on what mode the device was in. I made some good progress, but I was having some issues with it updating consistently with the selected mode. I like the idea of a popup card instead though... hmmm.
But yeah, thanks for the feedback! Got a bit more life stuff to deal with, but hopefully I'll have some time soon to clean some of this stuff up for ya :)
@dalinicus I'm glad you did not take my feedback as picking your work apart because your integration has been of HUGE help to me and it's really appreciated. Everything was more of suggestions/feedback than anything else. I understand how something like going from speed
to level
would be a breaking change but I figured that I'd mention those things now instead of later just in case you do decide to make the change. There's been so many time that I thought I thought of everything but haven't and had to go back and make name changes. That now just made me think if keeping to a standard port_001
, port_002
, etc. for each device would work better. Is that how you designed it or does it come from the API as the name the user gives it in the UIS app? For example, if I have a four port controller and no longer need the humidifier and choose to replace it with an AC outlet switch to control a third party dehumidifier how would your integration handle that? If ports opposed to a device name were reported than I assume the transition inside of the integration would me smoother.
That reminds me of something else. Is is possible to add multiple devices for multiple controllers connected? For example, I was no longer using a UIS controller and unplugged it and deleted it from the app however all of the entities still remained in the integration. I was able to do a quick search to find and remove the unused entities but if there was a separate device for each controller then deleting one would be easier. I think the flow would be better too because the devices and it's entities from one controller would not be mixed in with those of another.
I wish I understood the code more and might even give it a try to maybe help make things easier on you especially when it's an integration that I already use so much.
And yes, I also keep on debating on if I should change the card to a popup because the current card takes up so much space on the page. One idea I thinking about is to try and replicate what AC Infinity does with the main page of their app that shows all connected controllers. I don't know how that applies to the coding but you might want to consider how multiple UIS controllers connected applies to creating a card.
One thing that I love about your integration is that I was able to add to my card the ability to quickly change the min/max level of the selected device. I hate how the UIS app makes me change to the OFF or ON mode just to change those levels. It works against me when I want to change the max level of a light during the time when the light is off. This means I have to either wait for the light to turn on and make the change or change the mode to ON and make the change and turning the light on during it's off cycle is obviously not good. Your integration allows me avoid that and easily make that change. I something I tend to frequently adjust which is why its at the top of the card regardless of selected mode.
Just to throw this out there, is there a way to utilize the app's automation? The modes have always been good enough for me but recently I needed a little more control. I used the app's automation feature and created one automation to control a daytime temperature and humidity and another to control a nighttime temperature and humidity. This might be too much to add but is there a way to somehow add a state entity that shows if controlling a certain device is not an option because an automation is already assigned? So on the app, if an automation is assigned to the device on Port 1 the mode screen will show a message that says that Modes are unavailable due to it being part of an automation. I think with your integration it now just throws back an error message.
I'll work on a popup design to see what I come up with and then post it to this thread. And again, I appreciate all the work you did in making this integration. And I apologize for any mistake in this, I wrote this rather quickly. And I hope your family emergency was resolved with a positive outcome.
@dalinicus I created a popup version of the card but still need to do some work on the card that triggers the popup. It already has the temp, humidity, and VPD state but I also want to add sensors to show which device is running and maybe a countdown timer that shows any upcoming state changes (dependent on selected mode).
I also found a way to hide the lag/delay when it comes to the state of the mode updating. I created an input_select
and listed all the modes in it and then used input_select.select_option
as the button action on the popup to change the selected mode on the list. That change in state triggers an automation which then uses select.select_option
to change the actual selected mode in the app. This hides any delay because the state shown on the popup is from the list not the actual integration/app.
I still can't figure out there's a delay because the length of it seems to change but so far here's what I have noticed. So there seems to be two different delays. The first delay is short and only a couple of seconds. If the state doesn't change then the delay seems to the length of time until the sensors update which based on what is set in the configuration. It seems almost as if instead of the transfer of data happening at the same time it happens separately. So the change is sent to the app but in order for the change to be seen in the integration the app has to send the change in state back to it. That's the short delay but sometimes that doesn't happen. When it doesn't the state change is not seen until the next sensor update which is dependent on the length of time it's set to in the configuration.
I definitely like the popup version better than having everything on the card.
Wow. That's looking amazing. Keep me posted on your progress. Is this something you can share and be included in the repositroy for others (either via code updates, yaml configuration, or maybe a readme file)?
And yeah, the delay in updates might not be the integration itself but possibly related to the AC Infinity API. Its finicky to say the least. Something I want to revisit when I have some time (hopefully real soon with the holidays coming up 🎉)
Was doing some refactoring, and I think I found why you're experiencing weird value update issues when switching modes. Looks like I missed a coordinator refresh on the mode select entity... which means values don't get true'd up until the next scheduled coordinator tick (aka refresh interval). Fixing the bug for next release.
Fixed in https://github.com/dalinicus/homeassistant-acinfinity/pull/40, including test coverage
@dalinicus I saw closed issue #45 and I know you said it was a Bluetooth issue but i think it might also affect others. I'm not sure if this could be fixed on your end but the VPD value defaults to PSI when the integration is installed in HA. In issue #48 you can see that PSI is being used. It should actually use kPa like in my screenshot below. I just made the change in the sensor's Unit of Measurement option and it now shows the correct value but if there's a way to automatically have it as kPa when the integration installs then that would be great. If not then maybe mention the need to make the change within the release notes.
@dalinicus I also noticed there's no option for Target under the Auto option only High/Low. I recently switched to Target in the app and realized that the option didn't exist in your integration. The app has both Target and High/Low triggers that can be used. It should probably be an easy add-on for you to do unless it causes some type of conflict.
@dalinicus I saw closed issue #45 and I know you said it was a Bluetooth issue but i think it might also affect others. I'm not sure if this could be fixed on your end but the VPD value defaults to PSI when the integration is installed in HA. In issue #48 you can see that PSI is being used. It should actually use kPa like in my screenshot below. I just made the change in the sensor's Unit of Measurement option and it now shows the correct value but if there's a way to automatically have it as kPa when the integration installs then that would be great. If not then maybe mention the need to make the change within the release notes.
The linked PR has nothing to do with this issue, as it was intended for a completely different integration by the same name that I do not maintain. That being said, HomeAssistant core changed something with how unit_of_measurement
and native_unit_of_measurement
work... I had a similar issue with another extension I maintain. Would you mind creating a separate issue thread for this, and I'll check it out?
example from the codebase. I assume I'll need to switch this to unit_of_measurement
instead.
@dalinicus I also noticed there's no option for Target under the Auto option only High/Low. I recently switched to Target in the app and realized that the option didn't exist in your integration. The app has both Target and High/Low triggers that can be used. It should probably be an easy add-on for you to do unless it causes some type of conflict.
I'm not sure what you're referring to when you say "Target" as I do not have any such field in my app. Can you create a separate issue thread for this, and provide screenshots of what you're referencing?
@dalinicus I also noticed there's no option for Target under the Auto option only High/Low. I recently switched to Target in the app and realized that the option didn't exist in your integration. The app has both Target and High/Low triggers that can be used. It should probably be an easy add-on for you to do unless it causes some type of conflict.
I'm not sure what you're referring to when you say "Target" as I do not have any such field in my app. Can you create a separate issue thread for this, and provide screenshots of what you're referencing?
I went back and looked and just realized that the Target option only appears for a humidifier. All it does is gives the user the most basic way to set a humidifier, the way in which most people know how to which is to just set a humidity level and that's it. It seems like when the device is a humidifier it triggers two subcategories to appear at the bottom, one is called Auto (which has the normal Auto High/Low options) and the other is Target.
If you don't see it try switching one of your port device types to Humidifier and see if it forces the option to pop up. It's not a necessity it's just an extra feature that if you get the chance to add it on then cool. I also don't know if it'll cause conflict with non-humidifier devices and idk if it's an option of only having it available when the device type is a humidifier. I need to learn the coding to help out with these things.
I also noticed that the app does something similar when certain devices are plugged into and outlet adapter. It seems like when an outlet adapter is connected the app will only display On and Off and no level option. I'm not sure if this matters or affects anything on the back-end of your app but I figure that I'd at least mention it just in case it does.
Below are screenshots of the app. I have Auto Mode selected and the subcategory of Auto selected in one screenshot and Target selected in the other.
On a side note, idk what AC Infinity devices you own but if you have one of their fans and depending on which one it is, they'll send you the new Gen 2 version of it free of charge. It has a bigger and better motor and some type of new Natural Wind levels that it can be set to. I have four Cloudray S6 Auto Oscillating fans and they're sending me four new Gen 2 versions free of charge. They're really good about stuff like that.
@dalinicus just wanted to let you know that everything is still working great. I redid the GUI for the controller a little. I'm constantly making changes or trying out new things. I know they plan to roll out an update soon for their app. I'm not sure what it will address but I know it'll address fan control for sure. Their new fans have the option to control the amount of oscillation and also has a mode called wind mode or something. It basically changes the level the fan is blowing as if it's real wind. Anyway, my question is if there is anyway add the type of device that is connected to each port. I think the options are fan, grow light, heater, humidifier. I'm also curious if there's anyway to incorporate the Dynamic Response options.
@almighty059 This is awesome! Would you provide it? even if it's not finished yet?
@BlushTTV it's not so much that it's not finished, it's more that I'm always adding or changing something based on my needs. I'm trying to figure out the best way to provide the code. For example, in the video you can see I have both a bar slider and a number box for adjusting the Max Level, I'm trying to see which one I prefer. I also use several cards from HACS including the Declutter Card because I have it templates for multiple units.
How many AC Infinity controllers do you have?
Would you prefer the controls appear in a popup window or just on the dashboard?
@almighty059 I think it's very well implemented, don't you make your own Git repo for it?
I think it's very well implemented. Do you create your own Git repo for this?
@BlushTTV no. I have never even made a Git Repo for it but I could.
@almighty059 That would be a dream! I really want to test this!
@BlushTTV I put the code in a repository. It might not be perfect but hopefully it will help. I will try to improve my explanation of it in the repo.
@almighty059 Thank you! I just looked at it and installed all the dependencies! Unfortunately it doesn't work, but I don't have a plan on how to install something like that. I'll do some testing later. Awesome job what you put together with all the addons!
@BlushTTV if you respond on my repo I'll help you out. I don;t want to fill up what @dalinicus is doing with a bunch of questions about it. And I assume it's my use of the decluttering card and sending variables that makes things so confusing.
Before I begin, I just wanted to say thank you for putting the time into creating this integration. It's made my life so much easier because now I can integrate 3rd party devices with my AC Infinity system. I have an AC Unit, dehumidifier, exhaust fan, secondary sensors, and more all using various automation and scripts inside HA to make everything runs smooth. I have found a couple small issues that you might want to fix as well as a couple of things to take into consideration if you're building a card. I made one HA with various cards but it keeps evolving. I have some basic coding knowledge but I'm lost as to how GitHub works or else I'd be trying to help you out. Hopefully one day I have the time to figure out what a pull request.
In one of your previous updates you made the change to use
_status
be shown asPlugged In
orUnplugged
. That's fine but since it's abinary_sensor
the state shows ason
oroff
instead. That through me off a little when I was trying to get use the state in a template I made because I was checking forPlugged In
orUnplugged
.In the README file under Data Available you refer to "Power - Current Power supplied to the connected device" but you no longer use that. You've replaced it with
_current_speed
which I think is better but you just need to update that change in the README file.Also in regards to
_current_speed
I would consider changing it to something other than "speed" because it doesn't really apply when it comes to the lights, humidifiers, etc. My suggestion is to use what they use in the app which is "level". I think using_current_level
would apply to all of the devices better than_current_speed
currently does.When changing the mode entity for one of the devices there is a delay based on what you have the polling set to. If I am polling new data every 5 seconds then it's not that bad however if I have it set to a minute then it can cause issues. I haven't had a lot of time to mess with it yet but I plan to later today. One of the issues that I have is that I use Conditional cards to only show the controls for the selected mode. The Conditional cards check the state of the mode entity and then display the associated settings for the selected mode. If I am polling data every 30-seconds then it's possible that I have to wait 30-seconds for the Conditional card to see the new state and show the correct card and entities. I also noticed that when using an
entities:
card the drop down menu sometimes doesn't show in full and gets cutoff which I think is due to cards that am I using above and below it. Theentities:
card that I use to display and change the state of the mode also does not seem to update like it should but I need to mess with it a little more. I want to try creating my owninput_select
with all of the modes and then use that in place of the mode entity. This way the display will update correctly regardless of what the mode state is.On - Just like
_current_speed
I would consider changing this from_on_speed
to something like_on_level
. For example, the light that I have connected shows asnumber.tent_001_light_on_speed
and I think something likenumber.tent_001_light_on_level
works better.Off - The same as On Mode. I would consider changing this from
_off_speed
to something like_off_level
.Auto - There are eight entities for Auto Mode. Four of them enable triggers and the other four are the levels of the trigger. All of them are named the same and start with
auto
except for two of them. Just to keep them all uniform I would change_humidity_low_trigger
toauto_humidity_low_trigger
and_humidity_high_trigger
toauto_humidity_high_trigger
.Timer To On - Instead of using
_minutes_to_on
use_time_to_on
or_timer_to_on
because it could be minutes or hours.Timer To Off - Instead of using
_minutes_to_on
use_time_to_off
or_timer_to_off
because it could be minutes or hours.VPD - Seems okay to me and it uses a toggle switch to enable which works better than the
assumed_state
that Schedule Mode uses.Cycle - Instead of using
_cycle_minutes_off
or_cycle_minutes_on
use_cycle_off_duration
and_cycle_on_duration
because it could be minutes or hours.Schedule - Use a toggle switch instead of a
assumed_state
to show whether the start or end time is enabled. I think it feels more like the flow of the app. That's the way you did it in VPD and Auto mode so I would continue to do the same for Schedule Mode.Below is what the card on my dashboard currently looks like. I put the
off_speed
andon_speed
on each all of the views of the card for easier access. When I use the app and want to change the level of a device I have to leave the mode I am in and go to either the Off mode or the On mode to make the change. Then I have to make sure to remember to set it back to the correct mode. So having it available for each move makes things a lot easier. I also might change the card to a popup card because it takes up so much room on the dashboard. The tab card is working fine to change between each device but I don't like using the drop down menu to change the mode. I think I'm going to replace that with a small grid of buttons similar to the app.