micropython icon indicating copy to clipboard operation
micropython copied to clipboard

Setting parameters consistency

Open carlosperate opened this issue 9 years ago • 1 comments

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:

  • Radio module https://github.com/bbcmicrobit/micropython/pull/283
  • I2C frequency https://github.com/bbcmicrobit/micropython/pull/296
  • Accelerometer rate and range https://github.com/bbcmicrobit/micropython/pull/266
  • GPIO set pull https://github.com/bbcmicrobit/micropython/pull/304

carlosperate avatar Jun 10 '16 19:06 carlosperate

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)... :-)

  • Changing existing APIs can be painful. We'd need a strategy to make this as easy for us, the users and maintainers of existing code as possible.
  • We should be aware of context: sometimes its simpler, a better fit of the API's domain or just more elegant to do something inconsistent than the norm. I guess you might say there's a hierarchy with (IMHO) simplicity and ease of use at the top, with consistency high up the hierarchy but not an absolute. Our job, as developers is to use our experience and wisdom to make the right call and not to be too hung up on conventions and consistency.

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.

ntoll avatar Jul 08 '16 19:07 ntoll