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

SerialTransfer.bytesRead != SerialTransfer.packet.bytesRead when using callbacks #83

Open
PaulBouchier opened this issue Feb 6, 2022 · 3 comments
Assignees

Comments

@PaulBouchier
Copy link

Describe the bug
The example arduino sketch in pySerialTransfer/README.md does a loopback echo using this line:
for(uint16_t i=0; i < myTransfer.bytesRead; i++)
which leads me to think I can rely on myTransfer.bytesRead to tell how many bytes were read during myTransfer.Available(). However, that is incorrect when using Arduino callbacks, because Arduino callbacks are called from Packet.parse() inside of SerialTransfer.available(), and since Packet.parse() has not returned bytesRead, SerialTransfer.bytesRead still contains 0, because as far as it's concerned, the read has not finished yet.

To Reproduce
Run the attached two programs: main.cpp on Arduino, loopback_callback.py on a linux (or any) PC.
Observe the following output from Arduino:

SerialTransfer Loopback Test
In echo_cb
packet bytesRead: 40 SerialTransfer bytesRead: 0 packet status 2
Got a message ID: 0 sending back msg len - one of the following two values: 0 40
myTransfer.tick() received a message
In list_cb
Got a message ID: 1 sending back msg len - one of the following two values: 0 8
myTransfer.tick() received a message

Observe in main.cpp that handleRxMsg is called from the two callbacks echo_cb() and list_cb(), and line 41 prints the message above that it is sending back msg len - one of the following two values, and it lists 0 and 40, that being SerialTransfer.bytesRead and SerialTransfer.packet.bytesRead. Code supplied uses myTransfer.bytesRead per the example cited above, but if you uncomment the lines at 50, 51 and 54, and comment 47, 48 and 53 the program works, because it will then use myTransfer.packet.bytesRead.

Expected behavior
Two variables, both called bytesRead, should contain the same value. Or better yet, there should not be two variables - duplication of data is always bad and this is a case where it broke the code owing to inconsistency. Or maybe you should make more class internal variables private - it's pretty uncool allowing users to get in & mess with internal class variables. Not sure of the best solution here, but the environment that callbacks are called in is significantly different than the non-callback case - maybe that environment needs documenting.

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):
Ubuntu 20.04

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context
The python file is provided as loopback_callback_copy.py.txt and main.cpp also has a .txt extension, to enable uploading.

loopback_callback (copy).py.txt
ext**
main (copy).cpp.txt

@PowerBroker2 PowerBroker2 self-assigned this Feb 12, 2022
@PowerBroker2
Copy link
Owner

Is this still an issue in light of PowerBroker2/pySerialTransfer#50?

Also, I added callback functionality for completeness, but it might be more stable to process the packets normally (w/o callbacks).

@PaulBouchier
Copy link
Author

I think this is a documentation issue. I'm thinking a note in the README.md along the lines of "Note: when using callbacks on Arduino, the following SerialTransfer variables are invalid: SerialRead, & any others". Callbacks are really important functionality, and I greatly value it, because it means a sender can send a message and the right handler will get called on the other side to handle that message type. It helps encapsulation of message flows. If you want I'll propose some documentation.

I don't think this issue of proper callback use is related to PowerBroker2/pySerialTransfer#50 which is fundamentally about use of different types on the Arduino side vs. python size (float vs. double).

@PaulBouchier
Copy link
Author

Documentation updated in pull request #84

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

No branches or pull requests

2 participants