bbcmicrobit / micropython

Port of MicroPython for the BBC micro:bit
https://microbit-micropython.readthedocs.io
Other
607 stars 283 forks source link

Setting parameters consistency #306

Open carlosperate opened 8 years ago

carlosperate commented 8 years ago

I've opened this issue to continue a general discussion initiated in a Pull Request, that would probably have much broader effect.

We currently have init() methods for the SPI and UART modules, but not for the I2C. Apart from that there are other modules like the accelerometer or compass that could be configured using a similar pattern, but at the moment have different accessors for these properties.

Quoting @dpgeorge:

I think we need to have a broader discussion wrt setting parameters, to make sure things are consistent. Things that are configurable are:

GPIO: set pull (see #304), set analog period I2C: set frequency (see #296), change scl/sda pins SPI: phase, polarity, clock speed, bits, sck/mosi/miso pins UART: baudrate, bits, stop bit, parity, tx/rx pins accelerometer: set rate, range (see #266) compass: set sample rate radio: set freq, channel, address, etc (see #283)

It might make sense for some of these parameters to also have a "get", but probably not all of them.

Currently UART and SPI have an init() method that works as follows: the device starts up in a default mode (eg UART is used for the REPL) and calling init() is used to completely re-initialise the device. Any parameters that are passed are used, but any parameters that are not specified will take their default values.

One way to proceed is provide a generic config() method that takes keyword arguments to set a value, eg i2c.config(freq=100000); accel.config(range=8). You could get the value using i2c.config('freq').

Another way is to provide an individual method for each value to set (and perhaps get), eg i2c.set_freq(100000); accel.set_range(8).

Current PRs that could be affected by this are:

ntoll commented 8 years ago

OK... I'm consciously wearing my "devil's advocate" hat here. I want to make it clear that I believe consistency in API design is a desirable thing. However (putting my said devil's advocate hat)... :-)

As PEP8 says: "A Foolish Consistency is the Hobgoblin of Little Minds." ;-)

I don't like getter/setter functions in this Python/educational context. We should just do the Pythonic thing and use properties instead. I do like @dpgeorge's config suggestion but I wouldn't overload the function: rather I'd have a get_config("name") function. I also like a reset function to put everything back to a sensible state and having the config method update only those named arguments that are passed in. I'm neutral about mirroring method names to get us to set_config and get_config. I think long method names make it harder to type and thus raise another barrier to kids but I can see the aesthetic / consistency argument for them.

As to moving existing APIs from old -> new I suggest a time-boxed transition period where we keep both the old way while having the new way (whatever that might be) for some well advertised period of time. We can update the tools (Mu) and documentation to add warnings when appropriate. After X period of time remove the old ways all at once and also provide a very simple tool / facility (that could also be built into the tools) to help users convert old -> new (I can write these). Something like 2to3 but for MicroPython. ;-)

As always, thoughts, constructive criticism and ideas most welcome.

Importantly, I don't think this discussion should stop new work from happening. With EuroPython just over a week away and the attention of 1500 Pythonistas on us, we shouldn't stop radio and audio related work from going in - these are important features that'll inspire new members of our community to get involved and create resources and code. We can plan, migrate and document as soon as we find a way / heuristic to define the way forward.