pybricks
Init pattern for (singleton) objects in pybricks.common
So far, initialization of classes in pybricks.common has happened through functions of the form
mp_obj_t pb_type_Keypad_obj_new(uint8_t number_of_buttons, const pb_obj_enum_member_t **buttons);
Given the hub-specific properties, this returns the initialized object, which is then assigned to the hub.
This was intentionally different from the type->make_new MicroPython pattern:
mp_obj_t pb_type_Speaker_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args)
This fixed function signature makes this form less practical. We also don't generally need to expose the full type because no new instances can be made, at least not right now.
This issue is to settle on a useful pattern so we can keep it consistent as we expand the implementation of the Pybricks module.
It also occured to me that many of these (but not all) are singletons, so we could even ensure we allocate only a single object.
One benefit of the standard MicroPython pattern, although cumbersome for the way we use it today, is that it keeps options open to expose them to the user later. Say, for example, we might one day not only have the hub.speaker instance, but also sound_bar = Speaker(bluetooth_address).
I would say that the way things are now is fine. For objects that don't take any construction parameters, we can use still ->make_new without any added complexity. For constructors that need parameters, it is nice to not have to convert them to MicroPython objects and back if we don't need to. And I don't think it is worth the extra code size and complexity for just in case there may be some potential future use case.
For singletons, in the ev3dev port, I added a bool initialized field to the object struct to avoid multiple initialization, but I realized recently that we can just check if the type field has been assigned in the base object struct and do the same thing without adding an extra field.
We are using a third variant of this in the battery "class" by pretending it is a module. While this is a bit of a hack, this might be the smallest build size since we don't need to deal with an unused self argument.
It does not give any strange import behaviors so the only way this hack shines through is if you did type(hub.battery). We could apply this to more classes (e.g. keypad) if the savings are worth it.
I'm reopening this so we can pick a consistent pattern for releases after V3.2.
See also https://github.com/orgs/pybricks/discussions/1926. Initializing the hub multiple times is most likely an error, so maybe we should raise an exception.