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

Electron modem USART and buffer handling fixes #1140

Merged
merged 3 commits into from Nov 28, 2016

Conversation

@avtolstoy
Copy link
Member

commented Oct 17, 2016

Closes #1138 and #1104.

Fixes several issues:

  • USART_FLAG_TXE was not checked when writing into DR register from system thread which caused an unsent data byte to be overwritten
  • RX flow control was not correctly handled. When an internal ElectronSerialPipe buffer overflows, a data byte should be left in DR register to de-assert RTS line to ask modem to pause transmission for a while
  • ElectronSerialPipe circular buffer was initialized to 1024 bytes, but the implementation allowed only 1023 bytes to be held inside the buffer
  • AT+CLAC-like commands weren't parsed correctly (thanks @technobly)

Relevant tests:

  • MDM_01_socket_writes_with_length_more_than_1023_work_correctly
  • MDM_02_at_commands_with_long_response_are_correctly_parsed_and_flow_controlled

Doneness:

  • Contributor has signed CLA
  • Problem and Solution clearly stated
  • Code peer reviewed
  • API tests compiled
  • Run unit/integration/application tests on device
  • Add documentation (N/A)
  • Add to CHANGELOG.md after merging (add links to docs and issues)

Bugfixes

@technobly

This comment has been minimized.

Copy link
Member

commented on user/tests/wiring/no_fixture/cellular.cpp in 94e2894 Oct 17, 2016

Thanks for the great tests added with these @avtolstoy! Will the U260/U270/G350 all return 200 (or more) lines for this command? This coupled with the assert on RESP_OK will be plenty good in verifying that the command is completing. A follow up "AT" test command with RESP_OK would also verify that the modem is still responsive after this long command response, but not absolutely necessary.

@avtolstoy avtolstoy added this to the 0.7.x milestone Oct 17, 2016

@technobly

This comment has been minimized.

Copy link
Member

commented Nov 9, 2016

@avtolstoy any comment on my question above? Let me know if you need me to verify tests on a particular electron.

@technobly technobly self-assigned this Nov 21, 2016

@technobly technobly merged commit 99abe66 into develop Nov 28, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@technobly technobly deleted the feature/electron-mdm-usart-fixes branch Nov 28, 2016

@technobly technobly removed their assignment Nov 29, 2016

@technobly technobly modified the milestones: 0.7.x, 0.6.1 Nov 29, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.