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

Minor bugs involving flow, readme file, etc. and card ideas... #37

Open
almighty059 opened this issue Nov 1, 2023 · 24 comments
Open

Minor bugs involving flow, readme file, etc. and card ideas... #37

almighty059 opened this issue Nov 1, 2023 · 24 comments
Labels
enhancement New feature or request

Comments

@almighty059
Copy link

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 as Plugged In or Unplugged. That's fine but since it's a binary_sensor the state shows as on or off 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 for Plugged In or Unplugged.

  • 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. The entities: 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 own input_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 as number.tent_001_light_on_speed and I think something like number.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 to auto_humidity_low_trigger and _humidity_high_trigger to auto_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 and on_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.

Screenshot 2023-11-01 115115

@dalinicus
Copy link
Owner

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 dalinicus changed the title Minot bugs involving flow, readme file, etc. and card ideas... Minor bugs involving flow, readme file, etc. and card ideas... Nov 5, 2023
@almighty059
Copy link
Author

@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.

@almighty059
Copy link
Author

@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.

Untitled Project

@dalinicus
Copy link
Owner

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 🎉)

@dalinicus dalinicus added the enhancement New feature or request label Nov 20, 2023
@dalinicus
Copy link
Owner

dalinicus commented Nov 27, 2023

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.

image

Fixed in #40, including test coverage

@almighty059
Copy link
Author

almighty059 commented Feb 14, 2024

@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.

image

@almighty059
Copy link
Author

@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
Copy link
Owner

dalinicus commented Feb 15, 2024

@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.

image

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.
image

@dalinicus
Copy link
Owner

@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?

@almighty059
Copy link
Author

@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.

@almighty059
Copy link
Author

almighty059 commented Apr 22, 2024

@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.

ezgif-6-ed3bf74f8f

@BlushTTV
Copy link

@almighty059 This is awesome! Would you provide it? even if it's not finished yet?

@almighty059
Copy link
Author

@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?

@BlushTTV
Copy link

@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?

@almighty059
Copy link
Author

@BlushTTV no. I have never even made a Git Repo for it but I could.

@BlushTTV
Copy link

BlushTTV commented May 2, 2024

@almighty059 That would be a dream! I really want to test this!

@almighty059
Copy link
Author

@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.

https://github.com/almighty059/acinfinityhomeassistantcard

@BlushTTV
Copy link

@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!

@almighty059
Copy link
Author

@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.

@almighty059
Copy link
Author

@dalinicus so are you ready for a new project? lol

@dalinicus
Copy link
Owner

lol and here I just updated my tent to the Pro+ a couple months ago. The promise of a ph sensor though is enough to justify the upgrade for me. Pre-ordered :)

@almighty059
Copy link
Author

I thought about pre-ordering it but I'm going to wait. I want to see the probes and the setup first because I recently built my own environmental parameter monitoring system. This was the result of replacing the air stones in my RDWC system with a venturi. I wanted to monitor DO but my BlueLab is only for pH, EC, and water temp. After looking into DO meters and other monitoring systems I finally decided to try building my own. Based on what I've seen you do with the AC integration you might like this option.

I now use a Raspberry Pi 4B with sensors and probes from Atlas Scientific. They seem to make high quality products which range from consumer, lab, research, and industrial grade. Their probes can be fully submerged indefinitely and their sensors are also compatible with probes made by other companies. Their EZO sensors use UART or I2C to communicate and a fully configurable. They also offer RPi software that provides a UI for monitoring, calibrating, and some configuration options. I currently have pH, EC, and DO probes but I can add ORP, temperature, humidity, CO2, oxygen, water pressure, water temp, flow meters, float switches, and even dosage pumps. And since I'm using an RPi I I don't really have an limits on what else I can add.

If you're okay with pH and EC than the new AC system should be fine but if full automation is something you'd consider doing than you might want to look into this. I priced it out and you can have everything I mentioned for around $1500 (consumer grade) which I think is an extremely good price. Nuravine is one of the only companies that offers close to the same amount of options but it would cost around $4000.

@maziggy
Copy link

maziggy commented Oct 28, 2024

lol and here I just updated my tent to the Pro+ a couple months ago. The promise of a ph sensor though is enough to justify the upgrade for me. Pre-ordered :)

Same here two weeks ago. Damn.

@almighty059
Copy link
Author

@dalinicus do you know of another card besides the default one that can be used to select the time for the scheduled on and scheduled off entities? If it was an input_datetime there are other options but due to it being a time there doesn't seem to be any. Thanks.

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

No branches or pull requests

4 participants