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.

Sign up for GitHub

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

Jump to bottom

Translations and refactor #8

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Translations and refactor #8

merged 2 commits into from
Feb 4, 2025

Conversation

Delmael
Copy link
Collaborator

@Delmael Delmael commented Jan 16, 2025

Hi @ajtudela
Sounds way better !

I was working on translation and saw you've done the es part. The fr translation is in this MR.
There is also some refactor for entities (and tests updates for it).

Thanks

@codecov Codecov
Copy link

codecov bot commented Jan 16, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.13%. Comparing base (ded81c7) to head (789471d).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main       #8      +/-   ##
==========================================
- Coverage   96.44%   96.13%   -0.32%     
==========================================
  Files           9       10       +1     
  Lines         788      724      -64     
==========================================
- Hits          760      696      -64     
  Misses         28       28
Files with missing lines Coverage Δ
custom_components/smartbox/__init__.py 85.18% <100.00%> (ø)
custom_components/smartbox/climate.py 97.11% <100.00%> (-0.28%) ⬇️
custom_components/smartbox/config_flow.py 80.59% <100.00%> (-0.57%) ⬇️
custom_components/smartbox/entity.py 100.00% <100.00%> (ø)
custom_components/smartbox/model.py 98.90% <100.00%> (ø)
custom_components/smartbox/number.py 100.00% <100.00%> (ø)
custom_components/smartbox/sensor.py 98.96% <100.00%> (-0.24%) ⬇️
custom_components/smartbox/switch.py 100.00% <100.00%> (ø)

@ajtudela
Copy link
Owner

Thanks, I'll try to review this evening.

@ajtudela
Copy link
Owner

Hi, I've tested and it's working fine but there's a problem with the icon:
image

@Delmael
Copy link
Collaborator Author

Delmael commented Jan 28, 2025 •
edited
Loading

Hi, I've tested and it's working fine but there's a problem with the icon: image

Hi @ajtudela
This is related to the brand picture and the domain.
By default, for the integration and device pictures hass go to https://brands.home-assistant.io/[domain]/icon.png
My PR for smartbox is not yet approuved as its a product nor brand icon...

We have options:

  1. not doing anything : there will be the actuel miss icon
  2. changing the domain in our config file (for exemple heatger ). But there is a risk someone has already installed this integration and there will be a lot of errors.
  3. renaming the current integration to helki (which is the plateform behind everything) or heatingcontrol

@ajtudela
Copy link
Owner

I'm thinking of another option:
4. Add this icon to this repo and use it without dependencies from others.

@Delmael
Copy link
Collaborator Author

Delmael commented Jan 28, 2025

Well I don't think it's possible as this is home assistant which go get the ico
And I do not see where I can have a specific ico

@ajtudela
Copy link
Owner

Well I don't think it's possible as this is home assistant which go get the ico And I do not see where I can have a specific ico

Ok. So, the problem is the icon contains a product image. I think we should make our own custom icon for this integration and try again.

I saw many custom HACS integrations with icons not related to any brand or product.

@Delmael
Copy link
Collaborator Author

Delmael commented Jan 28, 2025

Maybe the one from https://www.amazon.co.uk/Valor-Desarrollo-Innovacion-Heating-Control/dp/B08422YBTY/ (juste the circle) ?
HeatingControl is the app which is required to used smartbox in google home/alexa (that's why I was wandering if we should not rename the smartbox integration name)

@ajtudela
Copy link
Owner

Maybe the one from https://www.amazon.co.uk/Valor-Desarrollo-Innovacion-Heating-Control/dp/B08422YBTY/ (juste the circle) ? HeatingControl is the app which is required to used smartbox in google home/alexa (that's why I was wandering if we should not rename the smartbox integration name)

I don't like renaming the integration because something like "heating control" is a general term that can be used in other devices, while "smartbox" is exactly the device used for these heaters. Another reason is that "smartbox integration" is a well known integration, if the name is changed it will confuse the users.

@Delmael
Copy link
Collaborator Author

Delmael commented Jan 28, 2025

Right! I understand 😃
It's too bad that aliasing is not easy !

(i've updated https://github.com/home-assistant/brands/pull/6347/files while there is some answer on the forum)

@ajtudela ajtudela merged commit bf94689 into ajtudela:main Feb 4, 2025
6 checks passed
@ajtudela
Copy link
Owner

ajtudela commented Feb 4, 2025

I have approved this so we can continue development, but the next release will have to have the icon.

@Delmael
Copy link
Collaborator Author

Delmael commented Feb 4, 2025

Hi
I totally agree with you.
New development are incoming :)
Thanks

Delmael added a commit to Delmael/hass-smartbox that referenced this pull request Feb 6, 2025
@Delmael
Translations and refactor (ajtudela#8)
066ff5a 
* add translations. DeviceEntity refactor. Tests updates.

* update tests for subscribe device.
ajtudela pushed a commit that referenced this pull request Feb 7, 2025
@Delmael
Samples, Resailers and updates (#9)
1dc9ac7 
* add translations. DeviceEntity refactor. Tests updates.

* update tests for subscribe device.

* select api and make configuration_url

* update const test

* async await everywhere. and ha>2015 required

* add home iin device

* requirements sync

* patch await

* add tests

* update tests

* remove init

* add tests

* miss error string

* add init tests

* regroup pyfixture

* add some entiticategory to config

* add pytest and gitignore junit

* pytest

* remove pytest.ini

* Add samples and history samples

* get always last day (not juste since last_stat)

* add tests

* remove return if samples is empty

* add the SmartboxResailer as source

* test config flow

* add custom logo for entities

* update typing and errors

* update model

* add translations. DeviceEntity refactor. Tests updates.

* update tests for subscribe device.

* select api and make configuration_url

* update const test

* async await everywhere. and ha>2015 required

* add home iin device

* requirements sync

* patch await

* add tests

* update tests

* remove init

* add tests

* miss error string

* add init tests

* regroup pyfixture

* add some entiticategory to config

* add pytest and gitignore junit

* pytest

* remove pytest.ini

* Add samples and history samples

* get always last day (not juste since last_stat)

* add tests

* remove return if samples is empty

* add the SmartboxResailer as source

* test config flow

* add custom logo for entities

* update typing and errors

* update model

* Translations and refactor (#8)

* add translations. DeviceEntity refactor. Tests updates.

* update tests for subscribe device.

* bump version

* bump version of smartbox

* update requirements common

* update requirements

* bump to python3.13

* add recorder in dependencies

* update manifest

* order by manifest

* Update README

* remove ignore brands information on hacs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Delmael @ajtudela