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

Very rough first draft of radio module related documentation. #305

Merged
merged 1 commit into from
Jul 11, 2016

Conversation

ntoll
Copy link
Contributor

@ntoll ntoll commented Jun 10, 2016

READY TO PROOF READ / MERGE

A "straw man" for the new radio module. Please annotate, comment and correct. Some of the details may need checking since this was written on a train so some of the specifications about the capabilities of the radio were from memory.

I'm not precious about any of the text I've written, so please be merciless. ;-)

Once we're happy with the API we can massage @dpgeorge's code found in #245.

It can be up to 256 (????? CHECK THIS from memory) bytes long.

The ``queue`` specifies the number of messages that can be stored on the
incoming message queue.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a circular queue? We should probably indicate what happens when receiving more messages than there are spaces left.

@markshannon
Copy link
Collaborator

See my comment about the API w.r.t bytes/strings #283 (comment)

@dpgeorge
Copy link
Member

See my comment about the API w.r.t bytes/strings #283 (comment)
...
I send a string, you recv a string. I send bytes, you receive bytes. Python is a dynamically typed language after all.
...

At the minimum we should provide a "raw" API whereby it's just bytes sent/received over the air, with no headers or anything added. Then people can use that to implement things however they like. On top of that we can, if needed, add a "friendly" API which does some pickling/unpickling of the data (eg using repr/eval) so that you can send typed data.

@whaleygeek
Copy link
Collaborator

whaleygeek commented Jun 24, 2016 via email

@markshannon
Copy link
Collaborator

@dpgeorge What does adding a "raw" protocol gain? If you want to send bytes, just send them.
Anyone can still write their own protocols and encoding on top of that.

The radio protocol already add channels, headers, data-whitening, lengths and check sums. So there really isn't such thing as raw data.

@dpgeorge
Copy link
Member

What does adding a "raw" protocol gain?

So you can communicate simply with non-Python code. The way I implemented it it already does "raw" bytes. Adding typed data is an extra layer on top of that.

If we eventually allow to control all the radio parameters then raw bytes are the only thing that makes sense to send.

@markshannon
Copy link
Collaborator

It doesn't have to be an extra layer. Bytes is merely one of many possible types that can be transmitted.
We can add the extra types alongside the bytes protocol, rather than on top of it.
There are 8 logical addresses. We can use one for bytes (as we do now) and the other 7 can be used for other types.

@ntoll
Copy link
Contributor Author

ntoll commented Jun 25, 2016

Thanks for the feedback so far. I've made some revisions in light of the conversation above.

@whaleygeek - I agree with what you're saying... leave it to the user to discover, explore and play. The most important thing we can do is make it easy and facilitate such playful exploration via a simple and obvious API.

I think it important that the API fulfil two functions (IMHO in order of importance):

  • It makes it as easy/simple/fun as possible for an 11yo kid to do wireless communication.
  • It is powerful/flexible/configurable enough for advanced users to do amazing things.

@dpgeorge @markshannon to fulfil the second requirement we should expose all the various configuration options (as I suggest with the config method) and allow access to the "raw bytes" level of sending a message. Given these two capabilities it should be possible for anyone to make full use of the device's radio capabilities.

Regarding the first requirement: it should be as simple as possible with the potential for playful exploration built in. From my point of view that simply means sending and receiving strings. Such messages would suffice for most basic use-cases. If an incoming message cannot be converted to a string it should be ignored (although it should be possible to override this behaviour) . It also means sending a message is three lines of code:

import radio

radio.on()
radio.send("Hello, World!")

Each line is easy to understand and tells an obvious part of the "story of using the radio" in a learner's mind.

Why not any other data types?

Because realising why we have different types and that they can be represented in different ways is something for kids to learn about, and this module is a good way to introduce some aspects of this area to them. It'll mean they encounter, first hand, the difference between "2" and 2.

Just using strings for the high level methods keeps the API very very simple.

Not doing anything else would also provide an excellent opportunity for a lesson in data serialisation (e.g. via repr) and a cool warning for why eval has problems (hint: radio.send("from microbit import panic; panic()")).

Finally, limiting messages to strings is both conceptually easy for a non-technical 11yo kid to understand (it's like writing a letter that's sent via the radio) yet limited enough that they'll soon realise why it's interesting to drop to the "raw bytes" layer to have more control over the messages being sent.

Does this make sense?

default values are sensible.

The ``length`` defines the size, in bytes, of a message sent via the radio.
It can be up to 256 (????? CHECK THIS from memory) bytes long.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can someone confirm what this upper limit is? I remember reading it somewhere but can't, for the life of me, find where that's referenced.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's 251 (254 - 3 bytes for S0, LENGTH and S1 preamble).

@deshipu
Copy link
Contributor

deshipu commented Jun 25, 2016

If an incoming message cannot be converted to a string it should be ignored (although it should be possible to override this behaviour)

Errors should never pass silently.
Unless explicitly silenced.

You are a very cruel person, if you want the kids to have to figure out why their messages are not being transmitted, even though everything should work. Especially since wireless transmission is involved, and there may be additional problems with range, power, channels, etc.

# To contain code that demonstrates the radio module.
import radio

radio.send("Hello, World!")
Copy link
Member

Choose a reason for hiding this comment

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

radio.on() first?

@ntoll
Copy link
Contributor Author

ntoll commented Jun 25, 2016

@deshipu you're right. I wasn't thinking things through (my head was in the "just make it work for the kids no matter what" space). Upon re-reading it's painfully obvious that errors MUST be reported and, to address @dpgeorge's point, it's a good opportunity to introduce the notion of try:except:finally: blocks.

I'll update the docs in light of the above and other recent documents.

@ntoll
Copy link
Contributor Author

ntoll commented Jun 25, 2016

OK... it feels like we're getting there. I'm certainly happy with the way the API is shaping up - it's certainly feels simple enough to get an 11yo sending wireless messages while still providing enough configuration and access to "raw bytes" to facilitate more complicated forms of communication.

Thoughts..?

@markshannon
Copy link
Collaborator

I really don't think that we need to abandon the "everything is an object" paradigm of Python that we use throughout the rest of the API, just for the radio module.
Everything in computing is composed of many layers from objects all the way down to transistors (and lower than that). We hide that.
Why say that the "bytes" layer in the radio module is special and must be exposed?

I'm sure that it is good for kids to understand that everything is composed of bytes.
But, I don't want the radio module to be unusable unless they have that understanding.
IMO, the audio module is probably a better place for that, as it must necessarily handle samples.

For interoperability with DAL based implementations is important that we can send and receive bytes, but strings and ints are more useful objects.

Much of the rest of the API is polymorphic, it keeps the API small and memorable.
The radio module should be the same.


.. py:function:: send(message)

Sends a message, expressed as a string or bytes. Sending a string is
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like "expressed as a string or bytes", I would prefer "which can be a string or bytes object". The message (in the context of the API) is an object.

Copy link
Member

@dpgeorge dpgeorge Jun 25, 2016

Choose a reason for hiding this comment

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

The message (in the context of the API) is an object.

The "message" should be a sequence of bytes, not an object. The ability to send a string (unicode) is a convenience thing.

@dpgeorge
Copy link
Member

I really don't think that we need to abandon the "everything is an object" paradigm of Python that we use throughout the rest of the API, just for the radio module.

Most (all?) the other communication entities in the API use bytes objects exclusively: uart, spi and i2c. It should be the same for radio.

You might want to add a layer to the uart/spi/i2c interfaces to send and receive arbitrary objects (eg i2c.send(42)) but first and foremost you should support transfer of raw bytes because that's how it works on the wire, and allows you to communicate with arbitrary devices.

@markshannon
Copy link
Collaborator

At no point have I suggested we don't support sending and receiving bytes. I just don't see why we need to make it unnecessarily difficult to send other objects.
My proposed interface has a strict super set of the capabilities of the one currently proposed, yet is more concise.
It is also (IMO) easier to understand and explain.
On 25 Jun 2016 12:51 p.m., Damien George notifications@github.com wrote:
I really don't think that we need to abandon the "everything is an object" paradigm of Python that we use throughout the rest of the API, just for the radio module.

Most (all?) the other communication entities in the API use bytes objects exclusively: uart, spi and i2c. It should be the same for radio.

You might want to add a layer to the uart/spi/i2c interfaces to send and receive arbitrary objects (eg i2c.send(42)) but first and foremost you should support transfer of raw bytes because that's how it works on the wire, and allows you to communicate with arbitrary devices.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

@dpgeorge
Copy link
Member

My proposed interface has a strict super set of the capabilities of the one currently proposed, yet is more concise.

I disagree with your proposal. I think that bytes should be special and at a lower layer, and typed data/objects at a higher level (and typed bytes objects can also exist at this higher level), building on the low-level bytes send/recv functionality. In particular you can choose to receive anything as bytes, even if it was sent as typed data (then you'd see the encoding scheme).

The logical address should be exposed to the user so they can write protocols that are compatible with C/C++ code that uses logical addresses in an arbitrary way.

So I would propose something like:

# raw bytes protocol
radio.send_bytes(bytes_obj)
bytes_obj = radio.receive_bytes()
# higher-level object protocol
radio.send(obj) # equivalent to radio.send_bytes(bytes(repr(obj), 'utf8'))
obj = radio.receive() # equivalent to obj = eval(str(radio.receive_bytes(), 'utf8'))

@ntoll
Copy link
Contributor Author

ntoll commented Jun 26, 2016

In the most recent push I renamed RADIO_MODE_MODE_Nrf_1Mbit -> RADIO_MODE_Nrf_1Mbit (too many MODEs).

Also, @dpgeorge I think kids may use the power settings. Evidence: a teacher was recently telling me how interesting it would be to measure the range of the devices given a remote-controlled robot project they had in mind.

@ntoll
Copy link
Contributor Author

ntoll commented Jun 27, 2016

+1 LGTM... I'll update the docs.

@ntoll
Copy link
Contributor Author

ntoll commented Jun 27, 2016

Actually, upon reflection, can we just make this string only for the moment.

@dpgeorge
Copy link
Member

dpgeorge commented Jul 8, 2016

There is still some ambiguity in the docs: if I call on(), then config(...), then off(), then on() again, does the second "on" reset the setting to the defaults, or use the last settings?

Stepping back a bit, I think it's a bit convoluted/confusing to have config() with no args reset all the state, and config(...) with a few args reset everything except the given args. I'd propose the following:

  • all radio settings are module-wide global variables and upon a fresh import of the module are set to their defaults
  • on() turns the radio hardware on, using the current settings, and does not change any settings
  • off() turns the radio hardware off, and does not change any settings
  • reset() resets all settings to their defaults
  • config(**kwargs) sets zero or more settings as given in the keyword arguments, but leaves the rest unchanged

@ntoll
Copy link
Contributor Author

ntoll commented Jul 8, 2016

@dpgeorge given the discussion today on the mailing list I was going to propose more-or-less the same. :-)

@dpgeorge
Copy link
Member

dpgeorge commented Jul 8, 2016

given the discussion today on the mailing list

Ah, yes, I see it now!

Any tweaks to my proposal? In #306 I suggested config('param') is used for get_config.

@ntoll
Copy link
Contributor Author

ntoll commented Jul 8, 2016

How about get_config('param') to be explicit and so the config function only does one thing?

@ntoll
Copy link
Contributor Author

ntoll commented Jul 8, 2016

BTW... my work from this morning is on ntoll/radio. Unfinished and with some documentation re-arrangement too so things a laid out more logically. YMMV but you can see where I was going with it.

@dpgeorge
Copy link
Member

dpgeorge commented Jul 8, 2016

How about get_config('param') to be explicit and so the config function only does one thing?

That makes it clear and explicit, but then "config" should be "set_config" (as per the mailing list discussion). Probably "reset" can stay as "reset". Note that the decision here is ultimately the decision for #306...

@deshipu
Copy link
Contributor

deshipu commented Jul 8, 2016

I don't know if that helps, but most Python style guides recommend that function and method names should be verbs or verb phrases, such as "get_configuration", "configure" or "reset", while attributes and properties should be nouns or noun phrases, such as "configuration" (they also advise against using shortened words or acronyms). However, Micropython seems to be developing this pattern where a method is used in place of a property (because property is inefficient), and where calling it with a parameter sets the property, while calling it without a parameter gets it.

There is also that recommendation that if an operation is costly (in time or otherwise) or has side effects, it should be a method, not a property. So the question is, does setting those properties have significant side effects or time penalties, or does it just set some internal registers, and the actual work is done when calling other methods?

By the way, is there any particular reason why it's just a single method to set all of the parameters, and not a separate method or property for each? It seems to me that this would make them easier to discover, especially with tab-completion. Is there a significant memory cost to this?

@ntoll
Copy link
Contributor Author

ntoll commented Jul 8, 2016

A while back, and after much discussion at both PyCon UK and on the mailing list, it was decided to use just method for the microbit related work. BBC based evidence suggested kids found these easier to understand and work with. Personally, I was on the "we should use properties, when they make sense" side of the fence but respect the decision of the group. It's evidence based, brings a certain consistency to things, and we've not had any feedback to the contrary of our decision except from programmers occasionally asking why we don't use properties. 😄

The micro:bit is but a first step. Those that take to it will dive into "wider" Python when given the chance. :-)

@ntoll
Copy link
Contributor Author

ntoll commented Jul 10, 2016

This documentation is ready to be proof-read and (hopefully) merged ASAP. It includes two additions to the docs:

  • Complete API documentation.
  • A teacher friendly tutorial that can be adapted for classroom use.

Happy to make modifications.

radio.rst
random.rst
spi.rst
uart.rst
Copy link
Member

Choose a reason for hiding this comment

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

Why are random, spi and uart now included twice?

@ntoll
Copy link
Contributor Author

ntoll commented Jul 11, 2016

OK... corrections and spelling mistakes sorted (it actually said "the best way to receive a massage" in the tutorial). :-)

Merge it!

@dpgeorge dpgeorge merged commit d9e5380 into master Jul 11, 2016
@microbit-carlos microbit-carlos deleted the radio-docs branch November 2, 2018 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants