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

UARTService: allow setting of buffer sizes, timeouts, and UUID #37

Open
dhalbert opened this issue Dec 1, 2019 · 12 comments
Open

UARTService: allow setting of buffer sizes, timeouts, and UUID #37

dhalbert opened this issue Dec 1, 2019 · 12 comments
Labels
enhancement New feature or request

Comments

@dhalbert
Copy link
Collaborator

dhalbert commented Dec 1, 2019

Add optional args to constructor to allow setting buffer sizes and timeouts for StreamOut and StreamIn objects.

@kevinjwalters
Copy link

Would be useful if the default value of the timeout(s) could be specifically mentioned in the documentation too.

@dhalbert
Copy link
Collaborator Author

dhalbert commented Dec 5, 2019

@tannewt: I started to work on this, but encountered a problem. The StreamIn and StreamOut are initialized as class variables:

class UARTService(Service):
    # ...
    uuid = VendorUUID("6E400001-B5A3-F393-E0A9-E50E24DCCA9E")
    _server_tx = StreamOut(uuid=VendorUUID("6E400003-B5A3-F393-E0A9-E50E24DCCA9E"),
                           timeout=1.0, buffer_size=64)
    _server_rx = StreamIn(uuid=VendorUUID("6E400002-B5A3-F393-E0A9-E50E24DCCA9E"),
                          timeout=1.0, buffer_size=64)

I initially made them instance variables, without thinking much, but then the binding doesn't happen. How can we set up customized versions for an instance so the users can set the timeouts and buffer sizes?

@tannewt
Copy link
Member

tannewt commented Dec 5, 2019

I think the easiest way would be to make them mutable after the init of CharacteristicBuffer. Once you have that, then you can set them in the init after calling super().init() which will do the binding.

@dhalbert
Copy link
Collaborator Author

dhalbert commented Dec 5, 2019

But since there's a single instance of each, if there's more than one UARTService, wouldn't they step on each other if they want different values for timeout or buffer_size?

@dhalbert
Copy link
Collaborator Author

dhalbert commented Dec 5, 2019

Or, do you mean change the properties of the CharacteristicBuffer objects? The buffer is allocated on instantiation, though, so we'd have to throw away the old buffer.

Is this inherent due to the descriptor protocol? Could the binding be restructured to use instances instead?

I'm worried there might be future cases where we can't make some aspect of a service mutable later.

@tannewt
Copy link
Member

tannewt commented Dec 5, 2019

There is a single instance of each StreamIn and StreamOut but they shadow themselves with either CharacteristicBuffer or BoundWriteStream for each service that is instantiated.

The timeout and buffer sizes could be removed from StreamIn/Out completely. They could be set on the service instance before calling super().__init__() and the binding could read them.

I thought about using instances for attribute objects but opted for binding descriptors so that the service definition was consistent for all attributes.

@kevinjwalters
Copy link

kevinjwalters commented Dec 15, 2019

Any rough ETA on this? I was using a longer timeout in a simple "protocol" and need to know whether to recode this or wait. I don't mind either way, btw.

It's ultimately for an Adafruit Learn Guide which I was hoping to finish up by 20th.

@dhalbert
Copy link
Collaborator Author

@tannewt Is this part of your addition of other kinds of buffers, or should it be done independently?

@tannewt
Copy link
Member

tannewt commented Dec 16, 2019

I don't have a pending change for this. PacketBuffer is independent at the moment.

@kattni kattni added the enhancement New feature or request label Apr 15, 2020
@dglaude
Copy link
Contributor

dglaude commented May 9, 2020

Would it be possible to do the same thing for the uuid?
For one of my BLE device I had to duplicated your code, only changing the uuid.
Do you want this in an other issue?

@dhalbert dhalbert changed the title UARTService: allow setting of buffer sizes and timeouts UARTService: allow setting of buffer sizes, timeouts, and UUID May 9, 2020
@dhalbert
Copy link
Collaborator Author

dhalbert commented May 9, 2020

@dglaude I just changed the title. Is this for the so-called "Transparent UART" service, with service UUID 49535343-FE7D-4AE5-8FA9-9FAFD205E455? I am using that here: https://github.com/adafruit/Adafruit_CircuitPython_BLE_BerryMed_Pulse_Oximeter/blob/master/adafruit_ble_berrymed_pulse_oximeter/adafruit_ble_transparent_uart.py, with just copied code, which I would like to fix eventually.

@dglaude
Copy link
Contributor

dglaude commented May 9, 2020

The UUID of the gadget I use is 53300001-0023-4BD4-BBD5-A6920E4C5653 and the 001 is replaced by 002 for TX and 003 for RX. That vendor has various set of UUID for different model and hardware revision. But there is always that 2=TX / 3=RX change in the UUID.

Apparently it is very common for old protocol that used to be build on Bluetooth SPP (Serial Port Profile) and have evolved to BTLE where there is no real standard way to do serial communication.

I did exactly as you did for the Oximeter, but I would be happy to deduplicate the code.

The rest of the protocol are messages terminated by a semicolon, so I'll try to make a readline that do end on that rather than newline character.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants