pybricks.common: Refactored Speaker API to be hub specific. #91
Conversation
|
Wasn't quite sure about the right package structure and naming. Any advice? Also, what's the purpose of |
There was a problem hiding this comment.
Looks good, thanks!
The module for the EV3 Speaker class doesn't really matter since it doesn't exist in MicroPython, so what you have done is fine. But for the SPIKE Prime version, lets just leave it where it was.
The stuff in conf.py might have been from some old hacks that we aren't using anymore. If removing all 3 lines there doesn't affect how the EV3 pages look when built, then I guess it is safe to remove, but we can address that separately.
src/pybricks/_common.py
Outdated
| @@ -370,116 +370,6 @@ def dc(self, duty): | |||
| """ | |||
|
|
|||
|
|
|||
| class Speaker: | |||
There was a problem hiding this comment.
This class should stay here and be modified to reflect what is implemented on SPIKE Prime.
| @@ -80,20 +79,6 @@ class Motor(DCMotor): | |||
| ) -> int: ... | |||
| def track_target(self, target_angle: int) -> None: ... | |||
|
|
|||
| class Speaker: | |||
There was a problem hiding this comment.
This class should stay here and be modified to reflect what is implemented on SPIKE Prime.
Just wondering, why do you want to keep Prime Speaker in
I might be blind, but when looking at the doc built by |
As I mentioned in the other issue, if we ever add support for NXT or if LEGO comes out with a new hub with a speaker, then it will be common.
It does look like the EV3 build was removed, so I guess it doesn't matter. |
|
@dlech How about now? |
src/pybricks/hubs.py
Outdated
| from .geometry import Axis as _Axis | ||
| from .ev3dev._speaker import Speaker as _EV3Speaker |
There was a problem hiding this comment.
It would be nice to keep these in alphabetical order.
There was a problem hiding this comment.
Yep, no problem. Forgot to hit the import optimisation shortcut. Fixed now.
The Prime/Inventor Hub Speaker allows only a subset of EV3 Speaker API. Hence, splitting the API up.
|
Thanks! |
The Prime/Inventor Hub Speaker allows only a subset of EV3 Speaker API. Hence, splitting the API up.