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

drivers: modem: bg9x: Has no means to update size of received packet. #38654

Closed
ycsin opened this issue Sep 19, 2021 · 7 comments
Closed

drivers: modem: bg9x: Has no means to update size of received packet. #38654

ycsin opened this issue Sep 19, 2021 · 7 comments
Assignees
Labels
area: Modem bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale
Milestone

Comments

@ycsin
Copy link
Collaborator

ycsin commented Sep 19, 2021

Describe the bug
This issue is to document an unresolved bug that I encountered when I first tried the ZephyrRTOS with the bg9x modem driver on my EC21 modm.

The current quectel-bg9x driver's recv isn't working as expected, as the recv's unsolicited cmd handler doesn't update the packet size for the socket, compared to the ublox-sara-r4 driver that does as a reference.

This is related to #34273, where the MQTT CONNACK acknowledgement from the broker can never be read by the offloaded socket's implementation, since the data to be read is always zero.

In earlier version of Quectel modems or its firmware, when the modem receives something, the modem reports the <connectID> without specifying the size. For the current quectel-bg9x driver to work on these modem, it is required to query the size of the received buffer using a separate AT cmd, likely in a work.

Some recent firmware (> 2020) for some of the modems have been updated to report the length together with the <connectID> if configured to do so. For these lucky modems with the correct firmware, it is possible to update the buffer size directly in the unsol recv parser just like the ublox-sara's one.

To Test

  1. Build mqtt samples on a board using the bg9x driver
  2. See error

Impact
bg9x driver will not be able to receive anything.

Additional context
I've tried to fix the issue last time, IIRC, MQTT QoS 0 works after the patch but QoS 1 likely fails and I forgot why. I have since move to use gsm_ppp modem.

The commit can be found here for whoever interested to pick up this, but it is very messy as it contains some CCLK and GNSS stuff. I came up with this with the help of @elliot-wdtl

Some important things:

  1. The old on_cmd_unsol_recv is modified to submit a work to perform modem_qird_query_work
  2. modem_qird_query_work will do the AT cmd to query the size of the received buffer and update the socket accordingly.
  3. This patch was done when I didn't really comprehend how things work.

Quectel TCP/IP application notes:

@ycsin ycsin added the bug The issue is a bug, or the PR is fixing a bug label Sep 19, 2021
@elliot-wdtl
Copy link
Contributor

Some recent firmware (> 2020) for some of the modems have been updated to report the length together with the if configured to do so.

This is interesting info. I'm working with bg95 and I have made quite a few changes which I haven't got to trying to upstream yet. I take advantage of configuring the modem to indicate the length so maybe this will be a problem if it's not available on lots of modems. I have also found that even if I configure it to indicate the length, it will not indicate the length on SSL sockets which is a sticky point for me. You also can't query the size of the receive buffer on SSL sockets so I have a different workaround.

@ycsin
Copy link
Collaborator Author

ycsin commented Sep 21, 2021

I have made quite a few changes which I haven't got to trying to upstream yet. I take advantage of configuring the modem to indicate the length so maybe this will be a problem if it's not available on lots of modems.

I guess for older firmware's users they will have to implement the AT+QIRD way, at least your current implementation will cover lots of newer firmware and modems, that is still a huge improvement to its current state, which doesn't work at all.

@cfriedt cfriedt added the priority: low Low impact/importance bug label Sep 21, 2021
@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Nov 22, 2021
@ycsin ycsin removed the Stale label Nov 22, 2021
@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Jan 22, 2022
@ycsin ycsin removed the Stale label Jan 22, 2022
@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Mar 24, 2022
@ycsin ycsin removed the Stale label Mar 24, 2022
@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label May 24, 2022
@ycsin ycsin removed the Stale label Jun 6, 2022
@github-actions
Copy link

github-actions bot commented Aug 6, 2022

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Aug 6, 2022
@rerickson1 rerickson1 added this to the future milestone Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Modem bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale
Projects
None yet
Development

No branches or pull requests

5 participants