Introduce API for granular (Sensor)Reader operation

[benthorner]

Original PR: #32

The current reader classes aren't optimal for some usecases.
For example, it's hard to "plug in" a SensorReader in this code:

class DataSource:
    def start(self) -> None:
        # prepare to poll for data

    def poll(self) -> Data:
        # return a single data sample (or fail)

    def stop(self) -> None:
        # close down the source

In particular:

1. The current classes make it hard to separate management of
   resources ("start" / "stop") from getting data ("poll").

2. The current classes mix CLI-oriented error handling (example)
   with the lower-level concern of just getting the data.

This PR reorganises some of the reader code to address (2) and
exposes more of these internals as a set of new "open", "close"
and "read_one" methods for more granular control to address (1).

[avaldebe]

I really appreciate this PR and see potential on the use case it implements.

I'm concerned about the additional complexity it introduce, so I ask you to be patient with me as I try to figure out a good balance between complexity and conciseness.

[avaldebe]

Is this internal class really needed?

[benthorner]

I use classes because they give meaning to the data being passed around. For example, what is a (buffer, obs_data)? It's not obvious without a class to clarify that it represents the data from a Reading.

That said, I don't feel strongly about it as this is an internal method. We could have this instead:

def _read_one(...):
    ...
    return (buffer, obs_data)

...

def __call__(...):
    ...
    (buffer, obs_data) = self._read_one(...)
    ...
    yield RawData(obs_data.time, buffer) if raw else obs_data

What do you think?

(the construction of RawData would also have to be duplicated in read_one)

[avaldebe]

what is the purpose of read_one?
If it is to read one observation, these exceptions should be addressed internally and ask the sensor for an new message (silently).

If it is meant to request one observation (and let the caller handle the retry),
then this test are appropriate.

I think that read_one should deliver one observation, independently of how many samples are requested from the sensor and discarded for different reasons.

If you want the caller to handle the retries, I could add a max_tries option to read_one and __call__. This way when the retry exceeds max_tries the exception will bubble up to the caller.

[benthorner]

If it is meant to request one observation (and let the caller handle the retry)

Yes, that's right: that's what I need it to do.

I had some questions, but then thought it would be better to share an example. I've appended some extra commits, which are what I think you're aiming for with max_(re)tries and read_one.

The penultimate commit isn't great. I'd argue that's why read_one and ultimately Streams are better, because it avoids confusion over when samples, interval and (now) max_retries are relevant. Given where we are, it would be clearer to drop that commit and for me to just do next(reader()) manually.

Anyway, how does it look?

[avaldebe]

Yes, this is what I had in mind. Thanks for the implementation.
I'll merge this PR as soon as I you confirm that you're done with it.

Thanks again,
Á.

[benthorner]

Should be ready for another look now...