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

Add parent reference to embedded containers #1711

Merged
merged 1 commit into from
Feb 3, 2023

Conversation

rytilahti
Copy link
Owner

Allows embedded containers to access data from other embeddeds or the main status.

@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2023

Codecov Report

Merging #1711 (20b162a) into master (6a71870) will increase coverage by 0.00%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master    #1711   +/-   ##
=======================================
  Coverage   81.74%   81.74%           
=======================================
  Files         191      191           
  Lines       17935    17937    +2     
  Branches     3845     3845           
=======================================
+ Hits        14661    14663    +2     
  Misses       2986     2986           
  Partials      288      288           
Impacted Files Coverage Δ
miio/devicestatus.py 91.74% <100.00%> (+0.15%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@rytilahti rytilahti merged commit c656903 into master Feb 3, 2023
@rytilahti rytilahti deleted the feat/parent_reference_for_embeddeds branch February 3, 2023 16:21
Copy link
Contributor

@starkillerOG starkillerOG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not make some changes to getattr of DeviceStatus to make sure a embeded container can acces the master/other embeded properties?

@rytilahti
Copy link
Owner Author

The embedded container can use _parent to access its parent, and then use _embedded to find other child containers. We could introduce a helper method to access embedded ones by name if that's what you are looking for?

@starkillerOG
Copy link
Contributor

@rytilahti I ment we could make it such that we can use getattr(self, "MapList__map_name_dict") for example from within another embeded status container.
And then maybe getattr(self, "Main__battery") for example to acces stuff from the main container from within a embeded container.

@rytilahti
Copy link
Owner Author

You can already access the parent as well as other embedded containers like this:

battery = self._parent.battery  # it's better to use attribute access over getattr
consumables = self._parent.consumables

main_brush_used = self._parent.consumables.main_brush
main_brush_used_another_syntax = self._parent.consumables__main_brush  # the one above is preferred

Is that not enough for the use case at hand?

@starkillerOG
Copy link
Contributor

@rytilahti fair, I think this will surfice, we will do it as you illustrated.
I do think mypy is going to complain about self._parent.consumables not existing, but we will deal with that later.

@starkillerOG
Copy link
Contributor

I tested this and it is working. I implemented it in the Roborock multi map PR: #1614

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

Successfully merging this pull request may close these issues.

3 participants