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

When ECHO is deactivated and try to send random AT command AT+CSQ for example using dce->at(). at() returns OK but the response is corrupted. (IDFGH-11753) #463

Closed
3 tasks done
franz-ms-muc opened this issue Dec 21, 2023 · 1 comment · Fixed by #472
Assignees

Comments

@franz-ms-muc
Copy link
Contributor

Answers checklist.

  • I have read the documentation for esp-protocols components and the issue is not addressed there.
  • I have updated my esp-protocols branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

Hello Franz,

I’ve found an issue in esp_modem_command_library.cpp

When ECHO is deactivated and try to send random AT command AT+CSQ for example using dce->at().

at() returns OK but the response is corrupted.

Here is the LOG:

D (70408) usb_terminal: 0x3fcf68c8   41 54 2b 43 53 51 0d                              |AT+CSQ.|
D (70408) CDC_ACM: Submitting BULK OUT transfer
D (70408) CDC_ACM: out/ctrl xfer cb
D (70408) CDC_ACM: in xfer cb
D (70408) usb_terminal: 0x3fc9a014   0d 0a 2b 43 53 51 3a 20  32 35 2c 39 39 0d 0a     |..+CSQ: 25,99..|
D (70418) CDC_ACM: Submitting poll for BULK IN transfer
D (70428) CDC_ACM: in xfer cb
D (70428) usb_terminal: 0x3fc9a023   0d 0a 4f 4b 0d 0a                                 |..OK..|
D (70438) CDC_ACM: Submitting poll for BULK IN transfer
I (70458) MODEM_COM_LIB: INTEGER ERROR: NO Match exits for -> �?ts�?25,99

As you can see beginning of the string is corrupted, this way we can’t correctly parse the command response.

I think that this is caused by usage of std::string_view inside generic_get_string().

It looks like the beginning of the data buffer is overrided between return of the lambda function and actual return of t->command().

Since std::string_view represents a view of the data(means keep only pointer) we must be sure that the object which contains the data is preserved and data is not changed until it is copied into the new string object.

Current implementation is really error-prone and need to be updated.

Currently I always activate ECHO(ATE1) but this is very bad workaround.

Can you please inform Espressif about this problem.

Maybe describing where the problem comes from will help them fix it faster.

@github-actions github-actions bot changed the title When ECHO is deactivated and try to send random AT command AT+CSQ for example using dce->at(). at() returns OK but the response is corrupted. When ECHO is deactivated and try to send random AT command AT+CSQ for example using dce->at(). at() returns OK but the response is corrupted. (IDFGH-11753) Dec 21, 2023
@david-cermak
Copy link
Collaborator

Thanks for the report! You're right, we should copy the string content here:

rather than use std::string_view assignment.

david-cermak added a commit to david-cermak/esp-protocols that referenced this issue Jan 3, 2024
Previously we used std::string_view, which pointed to the lower-layers
buffer which might have been reused for other asynchronous operations
before processing it, thus causing corruption of replies.

Closes espressif#463
david-cermak added a commit to david-cermak/esp-protocols that referenced this issue Jan 15, 2024
Previously we used std::string_view, which pointed to the lower-layers
buffer which might have been reused for other asynchronous operations
before processing it, thus causing corruption of replies.

Closes espressif#463
david-cermak added a commit to david-cermak/esp-protocols that referenced this issue Jan 15, 2024
Previously we used std::string_view, which pointed to the lower-layers
buffer which might have been reused for other asynchronous operations
before processing it, thus causing corruption of replies.

Closes espressif#463
david-cermak added a commit to david-cermak/esp-protocols that referenced this issue Jan 16, 2024
Previously we used std::string_view, which pointed to the lower-layers
buffer which might have been reused for other asynchronous operations
before processing it, thus causing corruption of replies.

Closes espressif#463
david-cermak added a commit to david-cermak/esp-protocols that referenced this issue Jan 16, 2024
Previously we used std::string_view, which pointed to the lower-layers
buffer which might have been reused for other asynchronous operations
before processing it, thus causing corruption of replies.

Closes espressif#463
david-cermak added a commit to david-cermak/esp-protocols that referenced this issue Jan 22, 2024
1.1.0
Features
- Added support for at_raw() command (ae38110, espressif#471)
- Added iperf test for PPP netifs (976e98d)
- Added test that performs OTA to exercise modem layers (f2223dd)
Bug Fixes
- Fixed OTA test to gracefully fail with no verification (1dc4299)
- Added C-API to configure APN (ce7dadd, espressif#485)
- Fixed AT commands to copy strings to prevent overrides (741d166, espressif#463)
- Fixed incorrect dial command format (0998f3d, espressif#433)
- Fixed documentation and example on creating custom device (577de67, espressif#452)
- Removed CI jobs for IDF v4.2 (d88cd61)
- Fixed OAT test to verify server cert and CN (edc3e72)
- Fixed set_pdp_context() command timeout (1d80cbc, espressif#455)
Updated
- docs(modem): Added description of manual test procedure (68ce794)
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

Successfully merging a pull request may close this issue.

3 participants