Change Highlights:
- Fix error check after `k_work_reschedule_for_queue`. A value of 1
means job was scheduled which was resulting in a ton of "<err>
wifi_eswifi: Rescheduling socket read error" logs getting printed
due to the erroneous check
- When using the B-L475E-IOT01A, attempts to use a TLS socket result
in a hang when socket offload is enabled so I'd like to have a way
to disable the option. To accomplish this, I I switched the
`CONFIG_NET_SOCKETS_OFFLOAD=n` Kconfig option from `select` to
`imply`.
- There was a missing `net_context_set_state()` call when
`CONFIG_NET_SOCKETS_OFFLOAD=n`. I applied the same fix from #30664
for this case to fix the issue.
Signed-off-by: Chris Coleman <chris@memfault.com>
Commit 95b916d104 ("drivers: wifi: esp32: fix reconnect issue")
switched from thread created at runtime to statically defined thread.
The difference is mainly visible for simple applications that use
CONFIG_NET_CONFIG_AUTO_INIT=y, where networking setup code is executed
before main() and any statically defined threads. All ESP32 events are
just queued and never handled, so conditions enforced by
CONFIG_NET_CONFIG_NEED_IPV4=y are never met (e.g. Zephyr networking
layer is never informed about being connected).
Switch back to thread created at runtime, which starts at the moment
when k_thread_create() is invoked. This allows ESP32 event processing to
happen just after ESP32 WiFi driver gets initialized and before Zephyr
network auto initialization code (CONFIG_NET_CONFIG_AUTO_INIT=y).
Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Device won't reconnect automatically even if
AP station is available. This fix adds the carrier event, indicating
that network is present again enabling DHCP bound event.
Also, internal wifi event callback was added into wifi
driver to enable proper event handling.
Update west.yml to bring exposed wifi event callback.
Signed-off-by: Sylvio Alves <sylvio.alves@espressif.com>
Fixes#33843
Usage of k_work object from within net_pkt results in undefined behavior
in case when net_pkt is deallocated (by net_pkt_unref()) before work has
been finished.
Use per socket k_work object (sock->send_work) to submit send work and
put net_pkt objects onto k_fifo (sock->tx_fifo). Add a helper function
esp_socket_queue_tx() for that, which will make sure that packets are
enqueued only when send work handler will be successfully submitted (so
that all packets are consumed/dereferenced).
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Recently WiFi ESP32 driver (utilizing WiFi radio in ESP32 SoC) was
introduced into drivers/wifi/esp32/ and it already caused confusion as
there was existing drivers/wifi/esp/ directory for ESP-AT
driver (utilizing external WiFi chip, by communicating using AT commands
from any serial capable platform). So question has arisen whether it is
good to merge both, while they are totally different drivers.
Rename ESP-AT driver to be placed in drivers/wifi/esp_at/, so that it is
easier to figure out difference between "esp32" and "esp_at" just by
looking at driver name. Rename also DT compatible and all Kconfig
options for the same reason.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
esp_reset() is called from net_if init function, which holds net_if lock
after commit 24b49f4399 ("net: if: Add locking"). At the end of
esp_reset() there is a blocking wait on `sem_if_up` semaphore. This
semaphore can be release only by esp_init_work(). esp_init_work()
however blocks on net_if operations, because net_if init function (which
invokes esp_reset() underneath) is still holding net_if lock. As a
result there is a deadlock, because esp_reset() and esp_init_work() are
both waiting on each other.
Remove waiting for `sem_if_up`, so that net_if init can exit and release
net_if lock.
Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Use recently introduced API, which takes care of gracefully closing any
pending DNS requests and replacing existing DNS server list with new
one.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
When BT and WiFi coexists, IRAM usage increases a lot.
Add configuration that allow wifi symbols
to be placed in flash, freeing space in IRAM.
Signed-off-by: Sylvio Alves <sylvio.alves@espressif.com>
Replace all existing deprecated API with the recommended alternative.
Be aware that this does not address architectural errors in the use
of the work API.
Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Several wifi drivers add some form of zephyr_include_directories(.)
which isn't needed since the headers in the specific driver dir get
included via #include "foo.h". So we can remove any uses of
zephyr_include_directories(.)
Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
There is a hardcoded macro in Winc1500 HAL SPI driver
that would prevent the driver from working.
This macro is now defined only based on KConfig entry.
This KConfig entry is by default not set.
To enable, set "CONFIG_WINC1500_DRV_USE_OLD_SW=y"
in proj.conf or board.conf
Signed-off-by: Raja D. Singh <rdsingh@iotwizards.com>
Convert esp driver:
NET_DEVICE_OFFLOAD_INIT -> NET_DEVICE_DT_INST_OFFLOAD_DEFINE
DT label is already required, so use it and drop CONFIG_WIFI_ESP_NAME
option.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
By the time hostname configuration was implemented, driver was switching
only between STA and STA+AP modes. After dynamic selection between NONE,
STA, AP and STA+AP was implemented (commit referenced below), hostname
configuration no longer takes effect when ESP chip obtains address over
DHCP (and sends hostname in the DHCP request).
Set hostname each time after enabling STA mode, so that it takes effect
in DHCP requests.
Fixes: 03ce61004b ("drivers: wifi: esp: control CWMODE depending on
current needs")
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
So far received packets were parsed (at AT command level) and allocated
in [esp_rx] thread. Then they were submitted to [esp_workq] thread for
processing (calling application callback).
This flow results in following deadlock when esp_workq thread waits on
response to some AT command:
- [esp_rx] waits on allocation of new RX packet
- [esp_workq] waits for [esp_rx] to process response to AT command
that was just sent
- blocked [esp_workq] prevents processing and deallocating RX packets
- [esp_rx] times out on allocation and closes socket
Process RX packets directly from [esp_rx] thread to prevent above
deadlock.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
ESP fetches DNS servers from local network by using DHCP. There is an AT
command to get those DNS addresses. Use that to provide DNS addresses
for Zephyr's DNS resolver.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Socket can be closed either by Zephyr or by peer. In the former case ESP
WiFi chip still notfies about closed socket, which currently results in
printing warning log:
<wrn> wifi_esp: Link X already closed
Change level of this log from warning to debug, so that driver users are
not concerned about situation that is a normal behaviour.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
There was a regression when implementing automatic AT+CWMODE{,_CUR}
handling based on driver needs. ESP AT 1.7 firmware does not support
AT+CWMODE_CUR=0, which means that radio needs to be either in STA, AP or
STA+AP mode (no NONE state available).
Fix ESP AT 1.7 compatibility by keeping radio in STA mode whenever it is
not used.
Move also first AT+CWMODE_CUR invocation before AT+CWDHCP_CUR, so that
the latter executes successfully with ESP AT 1.7.
Fixes: 03ce61004b ("drivers: wifi: esp: control CWMODE depending on
current needs")
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Added ap_enable and ap_disable api. The driver will open create an
access point with DHCP Server ip 192.168.1.1 and no security.
Added a small fix for the AF_INET issue.
Added parent and remote to accept routine context.
Added put implementation.
Signed-off-by: Nicolai Glud <nicolai.glud@prevas.dk>
net_context contains both net_sock_type and net_ip_protocol, which are
static during the lifetime of net_context. net_context has basically the
ownership of esp_socket, so we can be sure 'type' and 'ip_proto' are
always accessible through net_context API.
Remove 'type' and 'ip_proto' members from 'esp_socket' structure, as
those are already accessible by net_context API.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Change type of esp_socket->flags from uint8_t to atomic_t, so that read
and write access to those flags is done in atomic (thread-safe) manner.
Introduce esp_socket_ref() and esp_socket_unref() functions, which
operate on atomic refcount variable. esp_socket_ref() role is to
increase refcount if it was already non-zero. If it was zero then NULL
is returned, which means that socket is not used by net_context at the
moment.
Role of refcount:
* socket instance is assured to be between net_offload->get() and
net_offload->put() when refcount > 0,
* makes sure that socket instance can be used (its members can be
dereferenced) when refcount > 0,
* 'context' member is always valid and its members can be dereferenced
when refcount > 0.
esp_socket_get() gets unused socket, as previously. Additionally it sets
refcount to 1 at the end of call, which basically means that from that
point such socket can be referenced by other parts of the driver. Each
esp_socket_get() call should be followed by esp_socket_unref() and
esp_socket_put() to properly invalidate socket and prevent other parts
of driver from using it.
Add ESP_SOCK_WORKQ_STOPPED flag, which is now used to prevent scheduling
more work into driver workqueue. This flag is set in net_offload->put()
callback, so that no more socket work (such as processing RX/TX packets
or closing socket because of errors) is submitted after that.
Introduce mutex lock, which has following role:
* protects dst, connect_cb + conn_user_data, recv_cb + recv_user_data,
* assures that checking ESP_SOCK_WORKQ_STOPPED flag and actually
submitting (or not if net_offload->put was already called) new socket
work to workqueue is done in atomic way.
As there is a mechanism to prevent submitting new work items to
workqueue when net_offload->put() has been executed, then there is no
need to explicitly call esp_socket_ref() in esp_workq thread. This is
because one reference is being held by net_context (after calling
net_context->get()). This is why all the esp_socket_in_use() were simply
dropped. Code running from esp_rx thread on the other hand always uses
esp_socket_ref_from_link_id() helper function (which is backed by
esp_socket_ref()), so that it replaces previous esp_socket_in_use()
calls and additionally makes sure that socket stays valid ("in use")
until esp_socket_unref() is called.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
So far a dedicated FIFO was used for all RX packets, which was consumed
in single submitted work. This work was also responsible for closing
socket and notifying uppper network layers if some errors occurred
previously or socket was simply closed by peer. There is however a
potential race condition in scenario described below:
esp_rx thread | esp_workq thread
--------------------------|-----------------------------
| ---- esp_recv_work ----
| handle RX packets from FIFO
|
---- on_cmd_ipd ---- |
put new RX packet to FIFO |
---- on_cmd_ipd ---- |
|
---- on_cmd_closed ---- |
mark socket as closed |
---- on_cmd_closed ---- |
|
| handle close
| ---- esp_recv_work ----
In this case we assume that esp_workq was preempted just after
processing all RX packets from FIFO and before checking if socket was
closed. In such scenario RX packet put to FIFO just before doing close
is going to be unhandled, so application layer will miss part of the
data.
Change the way RX packets are scheduled to workqueue, by using the
already available net_pkt->work objects (used for example in native TCP
stack). Create a separate work for closing connection. As a result all
RX packets and close handlers are on the same queue and there is no risk
of handling close events before handling all previously received data.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
There might be some scheduled work related to socket currently requested
to be destroyed/closed. Schedule a dummy work to make sure all
previously running work items in workqueue are finished.
When talking about TX packets, this makes sure that all previously
scheduled data is actually sent (flushed) before closing socket.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
It is enough to initialize work structures once during driver init,
because work handlers do not change during driver lifetime.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Currently there are two code paths when sending packets:
asynchronous (using workqueue) when zero timeout was specified and
synchronous in other cases. This doesn't seem to be justified, so
convert code to always schedule packet sending using workqueue.
Each net_pkt has an embedded work item, so use it instead of esp socket
specific work that was shared across all sent packets. This gives a
possibility to schedule multiple packets for sending.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Callback and user data are saved in net_context structure. Those members
are used by native networking stack (net_if), so simply follow the same
pattern.
First of all this allows to reduce runtime information for driver socket
instance. Second and most important benefit is that it allows to move
send handling entirely to workqueue thread.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
ESP chip send number of available RX data using
+IPD=<sock>,<avail_bytes> command. This exact number (truncated to MRU)
was used to read data with AT+CIPRECVDATA=<sock>,<num_of_bytes>.
Use always MRU when sending AT+CIPRECVDATA=<sock>,<mru> request. When
there are less bytes available, then +CIPRECVDATA will just return less
bytes, which is fine for the driver.
There are two advantages to this new behavior:
* there is no need to follow how many bytes were notified by +IPD
message, thus reducing implementation size,
* when data is constantly received by ESP chip, then the last number of
bytes notified by +IPD is no longer up-to-date when sending a
AT+CIPRECVDATA; always requesting MRU number of bytes allows to
always receive maximum currently available number of bytes buffered
by ESP chip.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Dump of communication between ESP chip and Zephyr shows that
+IPD:<sock>,<bytes_avail> is always received after +CIPRECVDATA. This
means that we don't need to update sock->bytes_avail in esp_workq
thread. Additionally there is no need to schedule next AT+CIPRECVDATA
request, as that will be done by +IPD handler anyway.
Relying on +IPD to be received after each +CIPRECVDATA (as long as there
is some more data to be received) allows to simplify operations on
sock->bytes_avail. From now on only esp_rx thread will update its value
and schedule AT+CIPRECVDATA in esp_workq thread. Then in
sock->bytes_avail will be treated as "readonly" in esp_workq
thread. This allows to prevent race condition when both esp_rx and
esp_workq threads could potentially update value of sock->bytes_avail
value at the same time.
<sock>,CLOSED message is received always after retrieving all data from
ESP chip (using AT+CIPRECVDATA), so there is no need to check whether
there are more bytes to be received before marking socket as closed in
Zephyr driver.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Deduplicate final part of RX data processing for two supported cases:
passive (+CIPRECVDATA) and non-passive (+IPD). Move implementation to
esp_socket module, as this is strictly related to socket.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Split +IPD message handling into smaller logical functions, so it is
much easier to follow code and further improve it. Make sure to do as
much parsing work as possible, before accessing esp_socket object. This
will allow to introduce thread-safety locking just in the critical parts
in subsequent commits.
Prefer early return from function over goto to return instruction,
whenever cleanup code is not needed.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Current on_cmd_ciprecvdata handler is a bit overwhelming to follow.
Add a cmd_ciprecvdata_parse() helper, which is responsible only for
parsing received metadata (message offset and length), leaving socket
instance untouched.
Consume received payload later on, just after verifying that socket is
in valid state.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
After a synchronous (timeout > 0) connection attempt was made, it was
being checked if there is some packet to be sent. If positive, then a
send work was queued.
This behavior makes little sense, as TCP connection is not even
considered established at this point (from net_context perspective) if
function has not returned yet. Additionally it differs from
asynchronous (timeout == 0) connection attempt. In case of UDP (and
calling esp_connect() from esp_sendto()) sock->tx_pkt is assured to be
NULL. Drop this piece of code as it doesn't seem to be justified.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
2 (out of 3) components used CONFIG_WIFI_LOG_LEVEL, while the last one
didn't specify explicit log level. Always use CONFIG_WIFI_LOG_LEVEL in
all components.
While at it switch to LOG_MODULE_REGISTER(<module>, <log_level>), which
is a more compact way to define log level.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Calculate size based on the real message, instead of assuming some
arbitrary size. This consumes slightly less stack space, but more
importantly gives an idea what has been taken into account when
calculating the size.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
+IPD,<sock>,<bytes_avail> command in passive mode notifies about ESP
internal buffer usage level. Hence value of <bytes_avail> can exceed
4 digit value, which currently results in failure.
+IPD message is used in driver only for scheduling AT+CIPRECVDATA
command and actual value of <bytes_avail> doesn't really
matter. Increase maximum supported value that can be received from
9999 (maximum 4 digits) to 4294967295, which is maximum value of 32-bit
unsigned integer.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Set NET_CONTEXT_CONNECTED when stream socket got connected. This fixes
TCP connection when using eswifi WiFi driver, which got broken after
sockets layer started to validate net_context connection state before
allowing to receive any data.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Set NET_CONTEXT_CONNECTED when stream socket got connected. This fixes
TCP connection when using winc1500 WiFi driver, which got broken after
sockets layer started to validate net_context connection state before
allowing to receive any data.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Set NET_CONTEXT_CONNECTED when stream socket got connected. This fixes
TCP connection when using ESP WiFi driver, which got broken after
sockets layer started to validate net_context connection state before
allowing to receive any data.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
So far ESP chip was configured directly into STA mode. This works fine,
but consumes lots of power because of enabled WiFi radio, even when it
is not actively used.
Enter NONE mode during initialization, so WiFi radio will be
disabled. Switch between NONE, STA, AP and STA+AP modes depending on
what driver is currently doing (e.g. enable STA only when scanning,
connecting and being connected to AP).
AT+CWAUTOCONN=0 command fails when in NONE mode, so workaround that by
entering temporarily into STA and then switching back to NONE.
Add also a warning log when switching mode was not successful, to ease
debugging possible issues.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
AT+CWMODE command controls in which mode (NONE, STA, AP, STA+AP) ESP
chip is operating. Add helper macros to replace usage of magic
numbers (0-3), hence improve readability. Add also a esp_mode_switch()
helper function, which allows to conveniently switch to chosen mode.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Change internal API operating on ESP driver flags to support multiple
bitwise ORed flags at once. This allows to improve resulting code when
multiple flags are read or modified at once.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
All except one invocations of modem_cmd_send() pass the same interface,
command handler and semaphore. Create a helper function in order to make
invocations slightly more readable.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
Both 'cb' and 'user_data' parameters for send/sendto were saved as
'send_cb' and 'send_user_data' members in socket information. None of
them are actually used, so drop them.
Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
