Skip to content

Add support for a TextPacket, as sent using the UART module in the Ad… #33

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Dec 13, 2021
Merged

Add support for a TextPacket, as sent using the UART module in the Ad… #33

merged 9 commits into from
Dec 13, 2021

Conversation

TonyLHansen
Copy link
Contributor

Add support for a TextPacket, as sent using the UART module in the AdaFruit BlueTooth app

This code provides an exception case to the Packet creation logic, similar to how the sample Arduino BLE code for the LED Glasses also has an exception case when the UART stream has data that does not start with a "!".

@TonyLHansen TonyLHansen reopened this Nov 30, 2021
@TonyLHansen TonyLHansen mentioned this pull request Nov 30, 2021
Closed
FoamyGuy
FoamyGuy requested changes Dec 12, 2021
View reviewed changes
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Thanks for adding this feature @TonyLHansen! The changes to the library look good to me, and I was able to test them successfully with modified simpletest on:

Adafruit CircuitPython 7.1.0-beta.2 on 2021-12-10; Adafruit Circuit Playground Bluefruit with nRF52840
Board ID:circuitplayground_bluefruit

One request though if you are up for it: I think it would be great to include an example of using the new TextPacket type.

For testing I added this to the main loop inside of the simpletest script in the examples directory:

        if isinstance(packet, TextPacket):
            print("Received Text Packet:")
            print(packet.text)

I think we should either add that to the simpletest in the repo, or add a new example script that shows the TextPacket usage. This will make it easier for folks to find this new feature and see how to use it.

@TonyLHansen
Copy link
Contributor Author

@FoamyGuy, I decided to add a separate simpletest2.py example.

dhalbert
dhalbert requested changes Dec 13, 2021
View reviewed changes
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

I'm confused. The Bluefruit Connect app UART page does not send text packets with any kind of header. It simply sends raw text characters. (It has been a misconception in the past that it sent a particular packet type.) So there is no "text packet" type. But maybe I'm missing something.

@dhalbert
Copy link
Contributor

dhalbert commented Dec 13, 2021 •
edited
Loading

OK, I see that if you find unknown characters, you turn it into a TextPacket. One reason for not doing this in the past is that if you get out of sync, and miss a beginning "!" in another packet, then you'll read text as a TextPacket indefinitely until the data happens to have a byte that is a newline.

If we do want to send arbitrary text, then perhaps we should add a real text packet type, with a header, and update the app. Then we can send the packet with a length header or similar.

@TonyLHansen
Copy link
Contributor Author

TonyLHansen commented Dec 13, 2021 •
edited
Loading

@dhalbert, You are correct that it is possible for the packets to get out of sync if characters are missed.

However:

  • That is a true statement if characters are missed for any of the packet types.
  • Without code similar to this, the circuit python client code has to jump through hoops to get at the text data.
  • This class addition is still done in same way as the other packets, such that, if you don't want the XYZPackets, you don't import the class.
  • This class addition makes the circuit python client code easy to write.

Possibly most important from my point of view:

  • The arduino version of the "scrolling text" code does almost the exact same thing as this code is doing. That's where I got the ideas on how to handle the UART text.
  • It works with the current app just fine.

I don't disagree with your statement that updating the app to have a real "send text" capability would be useful. However I think that approach can be dealt with separately, and shouldn't prevent changes that work with the current version of the app.

@dhalbert
Copy link
Contributor

@TonyLHansen Thanks for the reply. I was missing the context of this change - I apologize if I seemed critical. Other people have asked for text packet functionality in the past, and were expecting a "!"-type packet.

How about we call this RawTextPacket or TextRawPacket, to distinguish its unusual data handling? If a regular text packet is added in the future, we can add a TextPacket class that does the usual header processing.

I would be fine with this PR after that.

dhalbert
dhalbert approved these changes Dec 13, 2021
View reviewed changes
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Thanks for your patience!

@dhalbert dhalbert merged commit d1c0a0f into adafruit:main Dec 13, 2021
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Dec 28, 2021
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@e796bc0)
8b88c77 
Updating https://github.com/adafruit/Adafruit_CircuitPython_HT16K33 to 4.1.8 from 4.1.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_HT16K33#80 from Daviey/moar_lolz
  > update rtd py version

Updating https://github.com/adafruit/Adafruit_CircuitPython_RA8875 to 3.1.8 from 3.1.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_RA8875#28 from adafruit/dhalbert-ustruct
  > update rtd py version

Updating https://github.com/adafruit/Adafruit_CircuitPython_SI5351 to 1.2.10 from 1.2.9:
  > Merge pull request adafruit/Adafruit_CircuitPython_SI5351#24 from adafruit/dhalbert-ustruct
  > update rtd py version

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_BroadcastNet to 0.11.0 from 0.10.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_BroadcastNet#23 from tekktrik/feature/add-sound-level
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_BroadcastNet#22 from tekktrik/feature/add-typing
  > update rtd py version

Updating https://github.com/adafruit/Adafruit_CircuitPython_BluefruitConnect to 1.2.0 from 1.1.10:
  > Merge pull request adafruit/Adafruit_CircuitPython_BluefruitConnect#33 from TonyLHansen/main
  > update rtd py version

Updating https://github.com/adafruit/Adafruit_CircuitPython_BoardTest to 1.2.8 from 1.2.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_BoardTest#19 from tekktrik/feature/add-typing
  > update rtd py version

Updating https://github.com/adafruit/Adafruit_CircuitPython_CursorControl to 2.5.1 from 2.5.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_CursorControl#30 from tekktrik/feature/add-typing
  > update rtd py version

Updating https://github.com/adafruit/Adafruit_CircuitPython_hashlib to 1.4.0 from 1.3.6:
  > Update README.rst
  > Merge pull request adafruit/Adafruit_CircuitPython_hashlib#17 from timhawes/main
  > update rtd py version

Updating https://github.com/adafruit/Adafruit_CircuitPython_Motor to 3.3.3 from 3.3.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_Motor#59 from FoamyGuy/typing_input_error_catch

Updating https://github.com/adafruit/Adafruit_CircuitPython_Register to 1.9.7 from 1.9.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_Register#46 from adafruit/dhalbert-ustruct
  > update rtd py version

Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 1.10.3 from 1.10.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#87 from tekktrik/main
  > update rtd py version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FoamyGuy FoamyGuy FoamyGuy requested changes

@dhalbert dhalbert dhalbert approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TonyLHansen @dhalbert @FoamyGuy