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

testing new Modem modem-v1.0.3 on a SIMCOM A7672E (IDFGH-11227) #375

Closed
3 tasks done
diplfranzhoepfinger opened this issue Oct 12, 2023 · 20 comments
Closed
3 tasks done
Assignees

Comments

@diplfranzhoepfinger
Copy link
Contributor

diplfranzhoepfinger commented Oct 12, 2023

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

testing new Modem modem-v1.0.3 on a SIMCOM A7672E

Test 1:

Test is with Firmware

esp> cmd AT+CGMR
I (1635107) modem_console: Sending command AT+CGMR with timeout 1000
I (1635117) modem_console: AT+CGMR
+CGMR: A011B09A7672M7_F

OK

esp> cmd AT+CSUB
I (1643567) modem_console: Sending command AT+CSUB with timeout 1000
I (1643577) modem_console: AT+CSUB
+CSUB: B09V02

OK

After adding some Delay and Sync, here it gets an IP Address. BUT only with INFO Logging, not with Verbose Logging.

CONFIG_LOG_DEFAULT_LEVEL_INFO=y log
CONFIG_LOG_DEFAULT_LEVEL_VERBOSE=y log

It is interesting to see that at "verbose" logging the behaviour is different than in "info" logging.
@david-cermak

Test 2:

testing the 4-Wire Mode: repo

Not working.

Example
CONFIG_LOG_DEFAULT_LEVEL_INFO=y simple_cmux_client log
CONFIG_LOG_DEFAULT_LEVEL_VERBOSE=y simple_cmux_client log
CONFIG_LOG_DEFAULT_LEVEL_INFO=y modem_console log
CONFIG_LOG_DEFAULT_LEVEL_VERBOSE=y modem_console log
Test 3:

Inflatable Buffer:

CONFIG_LOG_DEFAULT_LEVEL_INFO=y log
CONFIG_LOG_DEFAULT_LEVEL_VERBOSE=y log
Test 4:

cross-Testing with Old Modem:

repo

log

log

test with merged new Modem 1.0.3:

repo

log

@github-actions github-actions bot changed the title testing new Modem modem-v1.0.3 on a SIMCOM A7672E testing new Modem modem-v1.0.3 on a SIMCOM A7672E (IDFGH-11227) Oct 12, 2023
diplfranzhoepfinger added a commit to diplfranzhoepfinger/esp-protocols-tree-master-components-esp_modem-examples-simple_cmux_client_II that referenced this issue Oct 12, 2023
diplfranzhoepfinger added a commit to diplfranzhoepfinger/esp-protocols-tree-master-components-esp_modem-examples-simple_cmux_client_II that referenced this issue Oct 12, 2023
CONFIG_LOG_DEFAULT_LEVEL_INFO=y it works, CONFIG_LOG_DEFAULT_LEVEL_VERBOSE=y it fails espressif/esp-protocols#375
@david-cermak
Copy link
Collaborator

@diplfranzhoepfinger Does it mean that it works with the inflatable buffer? Looking at the logs, your'e hitting only the exit CMUX problem, which is related to the known issue of SIMCOM A7672

I think you're actually seeing this problem:

// cannot inflate and the processing hasn't finishes in the first iteration -> report a failure
command_cb.give_up();

I was thinking about adding a debug/warning log here, but this could also mean a standard timeout, so decided against it.

I know that I told you to set ESP_MODEM_CMUX_DEFRAGMENT_PAYLOAD to false in the past, because your device uses a longer CMUX payload and esp_modem wasn't ready to handle that.
It is also documented:

The operation would work without an issue in data mode, but some replies
in command mode might come fragmented in rare cases so might need to retry
AT commands.

that this mode is perfectly suited for DATA mode, but in the COMMAND mode, you might see get some timeout issues and this is probably the case. And it also explains why the logging verbosity makes such a difference, clearly sending long hexdump payloads to UART causes delays, so the data come in chunks and if the OK delimiter is not present in the first chunk we have no other option (in this configuration) than to give up and report failure.

Now the esp_modem was fixed to properly work in the default mode, as well. You can give it a try. (the data mode might be a bit slower, since we keep and copy buffers between CMUX and DTE layers). This config also handles corner cases, especially with your device A7672, so It would give a warning if the defragmenting bufffer size is not big enough:

ESP_LOGW("CUMX", "Failed to defragment longer payload (payload=%d)", payload_len);
// If you experience this error, your device uses longer payloads while
// the configured buffer is too small to defragment the payload properly.
// To resolve this issue you can:
// * Either increase `dte_buffer_size`
// * Or disable `ESP_MODEM_CMUX_DEFRAGMENT_PAYLOAD` in menuconfig


That said, I would recommend these configuration of esp_modem to work with A7672:

  • ESP_MODEM_CMUX_DEFRAGMENT_PAYLOAD=y, ESP_MODEM_USE_INFLATABLE_BUFFER_IF_NEEDED=n -- default, optimized for "embedded" usecase, static buffer on CMUX layer, may need bigger buffer size, slower networking
  • ESP_MODEM_CMUX_DEFRAGMENT_PAYLOAD=n, ESP_MODEM_USE_INFLATABLE_BUFFER_IF_NEEDED=y -- optimized for DATA throughput, buffer size could be small, but an allocation would take place if you get a long (or fragmented) command reply (which you do if you're using GNSS commands)

@diplfranzhoepfinger
Copy link
Contributor Author

ESP_MODEM_CMUX_DEFRAGMENT_PAYLOAD=n, ESP_MODEM_USE_INFLATABLE_BUFFER_IF_NEEDED=y

Set like you recommend

It does now at least get a IP Address; which was a big issue before.
But how can we go further?
We want not only get a IP but also get a Connection which seems not to be that case now.

Where does this "invalid header" warning come from?

@david-cermak
Copy link
Collaborator

But how can we go further?

This example terminates the connection in the end, so you either remove the exit or reconnect after it. Looking at your logs I can see that the connection and publishing to the MQTT broker succeeded -- in both verbose/info version.

Where does this "invalid header" warning come from?

I think we've discussed that already. Your modem doesn't comply to some control sequences of the CMUX protocol, I've even added a note about it in the docs:

3) Device A7670 does no not correctly exit CMUX mode. You can apply
this patch to adapt the exit sequence https://github.com/espressif/esp-protocols/commit/28de34571012d36f2e87708955dcd435ee5eab70

Previous version of esp_modem simply ignored it (the last command at the end didn't work), but I've recently added 8edbac6 to strictly check all violations, not only to emit warnings, but mainly to be able to recover the protocol. This proved very useful indeed, especially on UART if we miss an interrupt or buffer overflows. Very much applicable to devices like A7672, which define two byte payloads, so if such a glitch happen and is interpreted as a very long payload (like 32kB) we'd keep reading on and on, looking for the end of the frame (and CRC) which never comes.

@diplfranzhoepfinger
Copy link
Contributor Author

so i will integrate your patch and make new Tests. 

i do not think that anything runs in the current Version. The Device get an IP, but furthere communication does not happen. 

i will do more tests later today, and post them here. 

Thanks, 

Franz

diplfranzhoepfinger added a commit to diplfranzhoepfinger/esp-protocols-tree-master-components-esp_modem-examples-simple_cmux_client_II that referenced this issue Oct 17, 2023
diplfranzhoepfinger added a commit to diplfranzhoepfinger/esp-protocols-tree-master-components-esp_modem-examples-simple_cmux_client that referenced this issue Oct 17, 2023
@diplfranzhoepfinger
Copy link
Contributor Author

diplfranzhoepfinger commented Oct 17, 2023

first Example

repo with fix

                                                                                                                 
CONFIG_LOG_DEFAULT_LEVEL_INFO=y     log
CONFIG_LOG_DEFAULT_LEVEL_VERBOSE=y log                                                                            

that part looks good now ... wow.

but my Second Example lacks:
see:
repo with fix

                                                                                                                 
CONFIG_LOG_DEFAULT_LEVEL_INFO=y     log
CONFIG_LOG_DEFAULT_LEVEL_VERBOSE=y log                                                                            

seems also working, only Problem is the Fragmentation of the AT AT+CPSI? Answer.
can i increase the Default Buffer, so that is works in most cases without a Fragmentation ?

@david-cermak
Copy link
Collaborator

seems also working, only Problem is the Fragmentation of the AT AT+CPSI? Answer.

It comes fragmented, but should be passed to the command library multiple times, but the new fragments should be appended to the previous so in the end, it should arrive as a whole. This is also happening for other commands, that's why we parse them like this:

auto ret = t->command("AT+CMGS=\"" + number + "\"\r", [&](uint8_t *data, size_t len) {
std::string_view response((char *)data, len);
ESP_LOGD(TAG, "Send SMS response %.*s", static_cast<int>(response.size()), response.data());
if (response.find('>') != std::string::npos) {
return command_result::OK;
}
return command_result::TIMEOUT;
}, 5000, ' ');

(returning command_result::TIMEOUT to indicate that we need to read more, but after the next fragment arrives the buffer contains all accumulated data so far)

can i increase the Default Buffer, so that is works in most cases without a Fragmentation ?

Yes, as indicated above: #375 (comment) you can use the most default configuration (ESP_MODEM_CMUX_DEFRAGMENT_PAYLOAD=y, ESP_MODEM_USE_INFLATABLE_BUFFER_IF_NEEDED=n) to have the CMUX layer defragment the data. You can start with a smaller buffer and the library would tell you if you have to increase it.

@diplfranzhoepfinger
Copy link
Contributor Author

(returning command_result::TIMEOUT to indicate that we need to read more, but after the next fragment arrives the buffer contains all accumulated data so far)

so, how to get the Data then ? can i grab it somehow ?

can i increase the Default Buffer, so that is works in most cases without a Fragmentation ?

Yes, as indicated above: #375 (comment) you can use the most default configuration (ESP_MODEM_CMUX_DEFRAGMENT_PAYLOAD=y, ESP_MODEM_USE_INFLATABLE_BUFFER_IF_NEEDED=n) to have the CMUX layer defragment the data. You can start with a smaller buffer and the library would tell you if you have to increase it.

so you say maybe this config is the better one in some cases ?
Data Througput is less then you say ? Even with a increased Buffer ?

@diplfranzhoepfinger
Copy link
Contributor Author

Now I understand. We must change the Parser.

@david-cermak
Copy link
Collaborator

Now I understand. We must change the Parser.

Yes that's one option. The other one is to switch to the default config.

Data Througput is less then you say ? Even with a increased Buffer ?

I meant theoretical throughput, since the defragmentation takes place before we post the data to the network stack. This is of course negligible on UART speeds of few hundred kbauds.

@franz-ms-muc
Copy link
Contributor

thanks a lot, i will do a test.

@david-cermak
Copy link
Collaborator

@diplfranzhoepfinger You can also try to cherry-pick 1db1e15 to see if your parser works with ESP_MODEM_CMUX_DEFRAGMENT_PAYLOAD=n, ESP_MODEM_USE_INFLATABLE_BUFFER_IF_NEEDED=n.
Context: There's a regression in v1.0.3 with handling USB DTE data and this fix actually mimics the previous behavior for that config.

@franz-ms-muc
Copy link
Contributor

but we use A7672 on 4-wire serial. 

do you think USB has some siginficant advantages ? OK, it saves 2 Lines. 

but i still need CMUX right ?

@david-cermak
Copy link
Collaborator

but we use A7672 on 4-wire serial.

This also affects UART in the configuration of ESP_MODEM_CMUX_DEFRAGMENT_PAYLOAD=n, ESP_MODEM_USE_INFLATABLE_BUFFER_IF_NEEDED=n

do you think USB has some siginficant advantages ? OK, it saves 2 Lines.

Wires and speed

but i still need CMUX right ?

Not necessarily, A7670 supports two logical channels (endpoints) in USB, which could represent two physical channels (i.e. terminals) that you connect to the DTE. So you can have one channel for commands and another for data without multiplexing.

@franz-ms-muc
Copy link
Contributor

but i still need CMUX right ?

Not necessarily, A7670 supports two logical channels (endpoints) in USB, which could represent two physical channels (i.e. terminals) that you connect to the DTE. So you can have one channel for commands and another for data without multiplexing.

then i start ESP-Modem 2 times, once in DATA Mode, once in AT Mode ?

@david-cermak
Copy link
Collaborator

Just check the examples.
This would create the corresponding DTE with these two terminals already included:

ESP_LOGI(TAG, "Initializing esp_modem for the A7670 module...");
struct esp_modem_usb_term_config usb_config = ESP_MODEM_A7670_USB_CONFIG();
esp_modem_dce_device_t usb_dev_type = ESP_MODEM_DCE_SIM7600;

The rest is the exactly same (one DTE, one DCE, one network interface).


PS: This constructs a DTE which simply takes two terminals, and sets the mode to DUAL_MODE so no further switching is necessary.

DTE::DTE(const esp_modem_dte_config *config, std::unique_ptr<Terminal> t, std::unique_ptr<Terminal> s):
buffer(config->dte_buffer_size),
cmux_term(nullptr), primary_term(std::move(t)), secondary_term(std::move(s)),
mode(modem_mode::DUAL_MODE)

@franz-ms-muc
Copy link
Contributor

that is cool 

i want to test it !

@david-cermak
Copy link
Collaborator

It looks like the tests pass. Please reopen if not.

@diplfranzhoepfinger
Copy link
Contributor Author

YES ! 

we use it, and beside some initial Problems with the VBUS Pin ( USB still work when VBUS not on 5V, but Fail after this when activating GNSS) 

it is much more cool than using CMUX. 

so if somebody read this:

i would rate USB much better than CMUX.

@david-cermak
Copy link
Collaborator

I'm planning on enabling Discussions in this repo, so people can share what they think about different components and modes.
It's probably a good place for you to post a feedback on CMUX vs USB

@diplfranzhoepfinger
Copy link
Contributor Author

yep. might be a good thing !

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

4 participants