#Flexit Bacnet
wispy ferry
Boom!
covert mirage
Let me pull up your code real quick when I'm back at my pc
covert mirage
ah
Right
so since its a new itnegration
you have to use _attr_has_entity_name which you do
This means it will prefix the entity name with the device name
wispy ferry
I see!
covert mirage
For example for a device that has sensors it would be
{device} temperature
which makes snese
wispy ferry
Yeah, it does
covert mirage
but in your case you have you device set correctly
but
the name of the entity is the same
so swap
wispy ferry
Exactly
covert mirage
@property
def name(self) -> str:
"""Name of the entity."""
return self._device.device_name
with
_attr_name = None
With name set to None you are explicitly saying the entity name should be the device name
covert mirage
Sure, fire away
wispy ferry
sec...
covert mirage
left another small thing on the PR btw. I probably reviewed the PR when I was in the train r smth and I knew this was a thing but couldn't remember
wispy ferry
Okay, will check that out.
You wrote in the PR to inline the function validate_input do you mean as to remove the function and do the stuff in the if-statement directly?
covert mirage
Like, the validate_input function is something that is created by the scaffold script
and in many cases, the code that is executed is just a few lines
wispy ferry
Okay. Will remove the function. Now I saw your thing about PRECISION.
covert mirage
device = FlexitBACnet(user_input[CONF_IP_ADDRESS], user_input[CONF_DEVICE_ID])
try:
await device.update()
except (asyncio.exceptions.TimeoutError, ConnectionError, DecodingError):
errors["base"] = "cannot_connect"
except Exception: # pylint: disable=broad-except
_LOGGER.exception("Unexpected exception")
errors["base"] = "unknown"
else:
I fixed some of the stuff you commented on. Github confuses me though, think I somehow ended up reviewing my own PR.
wispy ferry
I agree
covert mirage
haha, you can press Review changes at the top right and click leave comment
wispy ferry
That is what I think I did 🙂
covert mirage
But I have to say, this was a very good first shot on a PR
wispy ferry
It was not the first shot. I'm continuing the work of another developer. I want this integration really bad since I have a flexit device. Also I'm a Javascript developer, not a python developer...
covert mirage
No but I mean, sometimes when people come up with these kind of PRs that are continuations and then it's still meh
do you want a script so you can run it on your own prod env?
covert mirage
cd /config
curl -o- -L https://gist.githubusercontent.com/bdraco/43f8043cb04b9838383fd71353e99b18/raw/core_integration_pr | bash /dev/stdin -d flexit_bacnet -p 104275
I think in a recent SSH update the /config dir was replaced with /homeassistant maybe check which one you need
wispy ferry
Oh, another question. I really dont understand the local dev env. Do I really have to restart home assistant every time I'm doing a change in the code? It is really frustrating.
Also it is not very well documented.
covert mirage
but this pulls the git repo, then copies the provided component and installs it as custom component
Yep
It's sadly not as flashy as vite or something
wispy ferry
What I'm used to 😉
covert mirage
Vue developer?
wispy ferry
React
covert mirage
ah
wispy ferry
Gatsby, CRA and NextJs
covert mirage
Ah nice
Yea I work with Vue in general
Recently worked on my first TS backend, which was also quite nice
wispy ferry
So compared to node land, python is... really messy.
covert mirage
I mean, the tooling is getting really nice tho
wispy ferry
I'm beginning to migrate from JS to TS in my projects at work.
covert mirage
Like I prefer any python tooling over eslint
Like Ruff, is a linter and formatter built in Rust
it formats
the whole code base
(!)
in 0.43 s
wispy ferry
The thing with JS is the developer experience. But python can run on a ESP8266 (!). Python is really nice with all libs and all -if you get it to run at all 😄
covert mirage
hahaha
covert mirage
Like, I still miss this with Javascript and all their kind of libs
But now I started with my internship and my goal is to make application configurable
And at this point I am really pushing some HA concepts like making structures with YAML config
wispy ferry
Good for you!
covert mirage
or json for that matter
wispy ferry
OMG
covert mirage
And at this point I am really pushing some HA concepts like making structures wi...
structures -> pages
wispy ferry
Now you lost me
covert mirage
Well, currently all our applications are custom made
wispy ferry
Yes
covert mirage
And they share some code, but its still custom
And we want consultants to be able to customize look and feel, and what information is displayed
but still being able to do something custom
And this is where I really like HA's way of being able to serialize your whole dashboard in YAML
wispy ferry
A hard problem I guess.
covert mirage
and you can use custom component types for example
wispy ferry
Yeah, for sure
I love HA. Been using it in production for five years now.
With ESP's and everything
covert mirage
Yep, it's amazing
Although my biggest gap is that I am now like a jehova's witness
when I walk in a friends home who doesn't use HA yet and I see 2+ things that can be connected
"May I tell you about our lord and saviour Home Assistant?"
wispy ferry
So I kinda understand HA from a users point of view, now I'm trying to understand it from a developers point of view... Steep learning curve.
Me too!
wispy ferry
"Have you heard about this thing called Home Automation"...
wispy ferry
Documentation?
covert mirage
I have seen people stop their development of stuff since they made one big PR to finish their one thing
and when a PR does a million more things, we want to split it up
and that takes a different way of looking to stuff
wispy ferry
Offcourse, that I know since I'm a developer.
I just want the integration merged so that we can have feedback and improve it over time
covert mirage
Right
covert mirage
But not everyone gets this, but this is the most important one
And I think the second thing, feel free to ask
covert mirage
Like, your device probably isn't special special, with this much integration, your use case already has been implemented somewhere
so always feel free to ask stuff
covert mirage
And I like to help out, so feel free to ask stuff whenever you feel like
wispy ferry
Side note: I'm already using the underlying library in a flask application in my house since a couple of months.
covert mirage
Ah nice
wispy ferry
Yeah, it it tested and works.
Anyway, I will push the changes now. You will probably get a ping via email. Thanks for now, really apreciate it!
wispy ferry
Ok, so I'm not trusted 😓
covert mirage
Like, I was bothering Frenck enough until he gave me the role to post images haha
covert mirage
He usually doesn't like that tho
wispy ferry
I got that feeling too 😉
covert mirage
Like, my traffic is still lowkey so I can just take these sidetrips to help people
wispy ferry
It was like two messages. Since then I havent bothered him.
covert mirage
but in his position, I get it
wispy ferry
Me too
So, you never answeded my question on the PR. Do you think we should remove async_turn_aux_heat_off?
covert mirage
oh I did not see that question
wispy ferry
...and the other aux heat things
wispy ferry
I could send you a screenshot of the question? 😉
covert mirage
Like I know that there is a discussion that it should be removed, but I don't know if there already is something in place to use now
wispy ferry
It does not seem like it, and other integrations use it.
covert mirage
Oh right
I think I know what this means
As I see this is only a turn on/off thingy
So how I now interpret this is, we should add a switch to do aux heating
(in a separate PR)
So I think it would be best to remove this for now
wispy ferry
Then I will remove the turn on/off aux heat as well.
And my last question. You wrote "...throw different parameters at your test...". Is this something I must do, or is it just a tip?
covert mirage
Like, python tests have 2 things that I really like
snapshot testing and parametrize
And I think it would cut down a lot in your testing code
And this is the point where I need to learn to give a dev some more freedom too 😛
wispy ferry
I dont really follow. Can you point me in the right direction (documentation) of how to do that and what that means?
covert mirage
there is no real docs about this btw
but in your case, your test_form_invalid_device, test_form_connection_error, test_form_decoding_error and test_form_unexpected_error are the same, except for the error added in side_effect and the error message checked in result["errors"] = ...
So this way you can make it 1 test, and add the raised errors as parameters of the test
this way, if you ever have to change some logic in the config flow, you only have to update it once
covert mirage
I have no clue actually
wispy ferry
Me neither
covert mirage
The thing is: We remove the auxiliary mode from the UI / more-info.
Thats in the proposal
wispy ferry
If I remove | ClimateEntityFeature.AUX_HEAT and the properties for aux we get an even smaller PR
Yeah, from the UI
covert mirage
I think we should not add it, as people might start to rely on it and then we will remove it
like, I would prefer to use the switch platform for this then
wispy ferry
All @sand wedgety for aux too?
covert mirage
And with the state its currently in, I think this is an easy merge
wispy ferry
FFS
covert mirage
haha no problem
Like, I don't think we should use it now and rather make sure we give the user the right ropes directly
wispy ferry
Removed it!
We can add a swith or whatever later.
At least now in the current state a user can 1: see the state of operation of the device and 2: change mode and temperature.
covert mirage
yep
wispy ferry
Since you don't get your emails. The PR bot has pinged you. Ping!
covert mirage
I think it looks mergable
The remark about the parametrizing would clean up a lot of tests
and since you're only starting a flow twice with that change, the flow_id and _patch_update wouldn't be used as much as it would now, so you could inline that as well
I think those are the only 2 things
and the fact that today is the beta cut off, so I am not going to merge it before the beta cut as that gives a bit more work
Oh and I just noticed mock_serial_number is only mocking the lib in the config flow. I think you can move that to the config flow (or just remove it as fixture and use it in the tests itself)
wispy ferry
Ok! Thanks for the feedback! I will look into that tomorrow.
wispy ferry
Oh no. I broke the tests...
wispy ferry
So, the thing is that the serial number was the title of the entity, but we changed that to be the device name. I tried mocking the device name as well but no luck. Pytest is kinda hard to understand... Will probably try something else tomorrow.
covert mirage
It looks hard to start with but it's quite doable tbh
Like, at first I was also wrestling a lot with it
lemme check what you patch
wispy ferry
Haven't committed anything. Don't want to embarass myself 🙂
covert mirage
haha, don't worry
wispy ferry
Also, I have to go in a moment. Maybe I can ping you tomorrow?
covert mirage
yea sure
I mean I am in the office, but I still have HA dev env setup 😛
I didn't even setup the env for their applications yet
But there is one thing I notice and that is that the error is coming from the library. And we like tests to completely patch the library so the only thing we test is HA code
wispy ferry
Exactly. Because now we miss a mock. There is an actual call to the library somewhere.
wispy ferry
Yeah, I mean (beleive) what we patch calls to the library.
wispy ferry
And we assert result2["title"] == "0000-0001" which was correct. But now the title is the device_name. We have to patch that too.
wispy ferry
I see. But, I might also be too tired at the moment 😉