From 743eb580de58b31a402ea6a2641fc45f7af053f4 Mon Sep 17 00:00:00 2001 From: Kasper Lund Date: Thu, 16 Jun 2022 13:57:30 +0200 Subject: [PATCH] Add WiFi close notifications (#799) * Start adding support for WiFi notifications * Add wifi closed notifications * Add is_closed to ethernet * Update gold * Reflow comment * Drop tabs --- lib/core/events.toit | 14 ++- lib/net/ethernet.toit | 3 + lib/net/impl.toit | 15 +-- lib/net/net.toit | 4 +- lib/system/api/network.toit | 6 ++ lib/system/services.toit | 3 + src/resources/wifi_esp32.cc | 50 ++++++---- system/extensions/esp32/wifi.toit | 95 +++++++++++++++---- .../gold/illegal_system_call_test.gold | 2 +- 9 files changed, 142 insertions(+), 50 deletions(-) diff --git a/lib/core/events.toit b/lib/core/events.toit index 8a16a0190..d93da56e3 100644 --- a/lib/core/events.toit +++ b/lib/core/events.toit @@ -12,11 +12,14 @@ monitor ResourceState_: group: return group_ resource: return resource_ + set_callback callback/Lambda -> none: + callback_ = callback + wait_for_state bits: return wait_for_state_ bits wait: - return wait_for_state_ 0xffffff + return wait_for_state_ 0x3fff_ffff clear: state_ = 0 @@ -29,6 +32,7 @@ monitor ResourceState_: unregister_monitor_notifier_ group_ resource_ resource_ = null group_ = null + callback_ = null remove_finalizer this // Called on timeouts and when the state changes because of the call @@ -36,8 +40,11 @@ monitor ResourceState_: notify_: resource := resource_ if resource: - state := read_state_ group_ resource - state_ |= state + state := state_ | (read_state_ group_ resource) + state_ = state + callback := callback_ + if callback: + callback.call state // Always call the super implementation to avoid getting // into a situation, where timeouts might be ignored. super @@ -55,6 +62,7 @@ monitor ResourceState_: group_ := ? resource_ := ? + callback_/Lambda? := null state_ := 0 read_state_ module id: diff --git a/lib/net/ethernet.toit b/lib/net/ethernet.toit index 80ce74400..3c6c42abc 100644 --- a/lib/net/ethernet.toit +++ b/lib/net/ethernet.toit @@ -166,6 +166,9 @@ class EthernetInterface_ implements net.Interface: return it.address unreachable + is_closed -> bool: + return not open_ + close -> none: if not open_: return open_ = false diff --git a/lib/net/impl.toit b/lib/net/impl.toit index c403728ff..2c362e197 100644 --- a/lib/net/impl.toit +++ b/lib/net/impl.toit @@ -20,7 +20,6 @@ open -> net.Interface: if not service: throw "Network unavailable" return SystemInterface_ service service.connect -// TODO(kasper): Find a way to listen for network closing. class SystemInterface_ extends NetworkResource implements net.Interface: // The proxy mask contains bits for all the operations that must be // proxied through the service client. The service definition tells the @@ -33,7 +32,7 @@ class SystemInterface_ extends NetworkResource implements net.Interface: super service handle address -> net.IpAddress: - if not handle_: throw "Network closed" + if is_closed: throw "Network closed" if (proxy_mask_ & NetworkService.PROXY_ADDRESS) != 0: return super socket := Socket try: @@ -45,13 +44,17 @@ class SystemInterface_ extends NetworkResource implements net.Interface: finally: socket.close + on_notified_ notification/any -> none: + if notification == NetworkService.NOTIFY_CLOSED: + task:: close + resolve host/string -> List: - if not handle_: throw "Network closed" + if is_closed: throw "Network closed" if (proxy_mask_ & NetworkService.PROXY_RESOLVE) != 0: return super host return [dns.dns_lookup host] udp_open --port/int?=null -> udp.Socket: - if not handle_: throw "Network closed" + if is_closed: throw "Network closed" if (proxy_mask_ & NetworkService.PROXY_UDP) != 0: return super --port=port return Socket "0.0.0.0" (port ? port : 0) @@ -61,14 +64,14 @@ class SystemInterface_ extends NetworkResource implements net.Interface: net.SocketAddress ips[0] port tcp_connect address/net.SocketAddress -> tcp.Socket: - if not handle_: throw "Network closed" + if is_closed: throw "Network closed" if (proxy_mask_ & NetworkService.PROXY_TCP) != 0: return super address result := TcpSocket result.connect address.ip.stringify address.port return result tcp_listen port/int -> tcp.ServerSocket: - if not handle_: throw "Network closed" + if is_closed: throw "Network closed" if (proxy_mask_ & NetworkService.PROXY_TCP) != 0: return super port result := TcpServerSocket result.listen "0.0.0.0" port diff --git a/lib/net/net.toit b/lib/net/net.toit index 9b86dd51c..841295ec4 100644 --- a/lib/net/net.toit +++ b/lib/net/net.toit @@ -17,6 +17,8 @@ open -> Interface: interface Interface implements udp.Interface tcp.Interface: address -> IpAddress + is_closed -> bool + resolve host/string -> List udp_open -> udp.Socket @@ -26,4 +28,4 @@ interface Interface implements udp.Interface tcp.Interface: tcp_connect address/SocketAddress -> tcp.Socket tcp_listen port/int -> tcp.ServerSocket - close + close -> none diff --git a/lib/system/api/network.toit b/lib/system/api/network.toit index 6c1e88334..d4a1122ce 100644 --- a/lib/system/api/network.toit +++ b/lib/system/api/network.toit @@ -35,6 +35,12 @@ interface NetworkService: static SOCKET_OPTION_UDP_BROADCAST /int ::= 0 static SOCKET_OPTION_TCP_NO_DELAY /int ::= 100 + /** + The notification constants are used as arguments to $ServiceResource.notify_ + and consequently $ServiceResourceProxy.on_notified_. + */ + static NOTIFY_CLOSED /int ::= 200 + // The connect call returns a handle to the network resource and // the proxy mask bits in a list. The proxy mask bits indicate // which operations the service definition wants the client to diff --git a/lib/system/services.toit b/lib/system/services.toit index b363a2be3..dd934c86f 100644 --- a/lib/system/services.toit +++ b/lib/system/services.toit @@ -253,6 +253,9 @@ abstract class ServiceResourceProxy: if _handle_ & 1 == 1: ServiceResourceProxyManager_.instance.register client_.id _handle_ this + is_closed -> bool: + return _handle_ == null + handle_ -> int: return _handle_ diff --git a/src/resources/wifi_esp32.cc b/src/resources/wifi_esp32.cc index d8fc813bb..5d96c5e10 100644 --- a/src/resources/wifi_esp32.cc +++ b/src/resources/wifi_esp32.cc @@ -36,9 +36,10 @@ namespace toit { enum { WIFI_CONNECTED = 1 << 0, - WIFI_DHCP_SUCCESS = 1 << 1, - WIFI_DISCONNECTED = 1 << 2, - WIFI_RETRY = 1 << 3, + WIFI_IP_ASSIGNED = 1 << 1, + WIFI_IP_LOST = 1 << 2, + WIFI_DISCONNECTED = 1 << 3, + WIFI_RETRY = 1 << 4, }; const int kInvalidWifi = -1; @@ -117,11 +118,12 @@ class WifiEvents : public SystemResource { FATAL_IF_NOT_ESP_OK(esp_wifi_stop()); } - uint8_t disconnect_reason() { return _disconnect_reason; } + uint8 disconnect_reason() const { return _disconnect_reason; } + void set_disconnect_reason(uint8 reason) { _disconnect_reason = reason; } private: friend class WifiResourceGroup; - uint8_t _disconnect_reason; + uint8 _disconnect_reason; }; class IPEvents : public SystemResource { @@ -135,7 +137,7 @@ class IPEvents : public SystemResource { free(_ip); } - const char* ip() { + const char* ip() const { return _ip; } @@ -164,7 +166,7 @@ uint32_t WifiResourceGroup::on_event(Resource* resource, word data, uint32_t sta state |= WIFI_DISCONNECTED; break; } - static_cast(resource)->_disconnect_reason = reason; + static_cast(resource)->set_disconnect_reason(reason); break; } @@ -175,34 +177,46 @@ uint32_t WifiResourceGroup::on_event(Resource* resource, word data, uint32_t sta case WIFI_EVENT_STA_STOP: break; + case WIFI_EVENT_STA_BEACON_TIMEOUT: + // The beacon timeout mechanism is used by ESP32 station to detect whether the AP + // is alive or not. If the station continuously loses 60 beacons of the connected + // AP, the beacon timeout happens. + // + // After the beacon times out, the station sends 5 probe requests to the AP. If + // still no probe response or beacon is received from AP, the station disconnects + // from the AP and raises the WIFI_EVENT_STA_DISCONNECTED event. + break; + case IP_EVENT_STA_GOT_IP: { ip_event_got_ip_t* event = reinterpret_cast(system_event->event_data); uint32_t addr = event->ip_info.ip.addr; - sprintf(static_cast(resource)->_ip, + sprintf( + static_cast(resource)->_ip, #ifdef CONFIG_IDF_TARGET_ESP32C3 - "%lu.%lu.%lu.%lu", + "%lu.%lu.%lu.%lu", #else - "%d.%d.%d.%d", + "%d.%d.%d.%d", #endif - (addr >> 0) & 0xff, - (addr >> 8) & 0xff, - (addr >> 16) & 0xff, - (addr >> 24) & 0xff); - state |= WIFI_DHCP_SUCCESS; + (addr >> 0) & 0xff, + (addr >> 8) & 0xff, + (addr >> 16) & 0xff, + (addr >> 24) & 0xff); + state |= WIFI_IP_ASSIGNED; break; } case IP_EVENT_STA_LOST_IP: + state |= WIFI_IP_LOST; break; default: printf( #ifdef CONFIG_IDF_TARGET_ESP32C3 - "unhandled WiFi event: %lu\n", + "unhandled WiFi event: %lu\n", #else - "unhandled Wifi event: %d\n", + "unhandled WiFi event: %d\n", #endif - system_event->id + system_event->id ); } diff --git a/system/extensions/esp32/wifi.toit b/system/extensions/esp32/wifi.toit index 47aa8f800..ce2a3a21d 100644 --- a/system/extensions/esp32/wifi.toit +++ b/system/extensions/esp32/wifi.toit @@ -64,7 +64,7 @@ class WifiServiceDefinition extends NetworkServiceDefinitionBase: try: module.set_ssid ssid password with_timeout WIFI_CONNECT_TIMEOUT_: module.connect - with_timeout WIFI_DHCP_TIMEOUT_: module.get_ip + with_timeout WIFI_DHCP_TIMEOUT_: module.wait_for_ip_address return module finally: | is_exception exception | if is_exception: @@ -76,9 +76,14 @@ class WifiServiceDefinition extends NetworkServiceDefinitionBase: class WifiResource extends ServiceResource: state_/WifiState ::= ? constructor service/ServiceDefinition client/int .state_: - super service client + super service client --notifiable + state_.wifi.add_resource this + + notify_closed -> none: + notify_ NetworkService.NOTIFY_CLOSED on_closed -> none: + state_.wifi.remove_resource this state_.down monitor WifiState: @@ -93,7 +98,13 @@ monitor WifiState: up ssid/string password/string -> WifiModule: usage_++ if wifi_: return wifi_ - return wifi_ = service_.turn_on ssid password + try: + return wifi_ = service_.turn_on ssid password + finally: | is_exception exception | + if is_exception: + // Do not count the usage if we didn't manage + // to produce a working $WifiModule. + usage_-- down -> none: usage_-- @@ -107,14 +118,18 @@ monitor WifiState: class WifiModule: static WIFI_CONNECTED ::= 1 << 0 - static WIFI_DHCP_SUCCESS ::= 1 << 1 - static WIFI_DISCONNECTED ::= 1 << 2 - static WIFI_RETRY ::= 1 << 3 + static WIFI_IP_ASSIGNED ::= 1 << 1 + static WIFI_IP_LOST ::= 1 << 2 + static WIFI_DISCONNECTED ::= 1 << 3 + static WIFI_RETRY ::= 1 << 4 logger_/log.Logger ::= log.default.with_name "wifi" resource_group_ := wifi_init_ + wifi_events_/monitor.ResourceState_? := null + ip_events_/monitor.ResourceState_? := null + ssid_ := null password_ := null @@ -124,11 +139,34 @@ class WifiModule: ssid_ = ssid password_ = password + // TODO(kasper): For now, we just keep these in a list. We could + // consider making the resource hashable to improve on this. + resources_ ::= [] + + add_resource resource/WifiResource -> none: + resources_.add resource + + remove_resource resource/WifiResource -> none: + // TODO(kasper): This is O(n), but we don't expect + // many resources. + resources_.remove resource + close: - if resource_group_: - logger_.debug "closing" - wifi_close_ resource_group_ - resource_group_ = null + if not resource_group_: return + + if wifi_events_: + wifi_events_.dispose + wifi_events_ = null + if ip_events_: + ip_events_.dispose + ip_events_ = null + logger_.debug "closing" + wifi_close_ resource_group_ + resource_group_ = null + + // Notify the resources' proxies that they have been closed. + resources_.do: | resource/WifiResource | resource.notify_closed + resources_.clear address -> net.IpAddress: return address_ @@ -138,19 +176,22 @@ class WifiModule: logger_.debug "connecting" while true: resource := wifi_connect_ resource_group_ ssid_ password_ - res := monitor.ResourceState_ resource_group_ resource - state := res.wait - res.dispose + wifi_events_ = monitor.ResourceState_ resource_group_ resource + state := wifi_events_.wait if (state & WIFI_CONNECTED) != 0: + wifi_events_.clear_state WIFI_CONNECTED logger_.debug "connected" + wifi_events_.set_callback:: on_event_ it return else if (state & WIFI_RETRY) != 0: + wifi_events_.clear_state WIFI_RETRY reason ::= wifi_disconnect_reason_ resource logger_.info "retrying" --tags={"reason": reason} wifi_disconnect_ resource_group_ resource sleep WIFI_RETRY_DELAY_ continue else if (state & WIFI_DISCONNECTED) != 0: + wifi_events_.clear_state WIFI_DISCONNECTED reason ::= wifi_disconnect_reason_ resource logger_.warn "connect failed" --tags={"reason": reason} close @@ -159,22 +200,34 @@ class WifiModule: if is_exception and exception.value == DEADLINE_EXCEEDED_ERROR: logger_.warn "connect failed" --tags={"reason": "timeout"} - get_ip: + wait_for_ip_address -> net.IpAddress: resource := wifi_setup_ip_ resource_group_ - res := monitor.ResourceState_ resource_group_ resource - state := res.wait - res.dispose - if (state & WIFI_DHCP_SUCCESS) != 0: - ip := wifi_get_ip_ resource - address_ = net.IpAddress.parse ip + ip_events_ = monitor.ResourceState_ resource_group_ resource + state := ip_events_.wait + if (state & WIFI_IP_ASSIGNED) != 0: + ip_events_.clear_state WIFI_IP_ASSIGNED + ip/string ::= wifi_get_ip_ resource logger_.info "dhcp assigned address" --tags={"ip": ip} - return ip + address ::= net.IpAddress.parse ip + address_ = address + ip_events_.set_callback:: on_event_ it + return address + else if (state & WIFI_IP_LOST) != 0: + ip_events_.clear_state WIFI_IP_LOST close throw "IP_ASSIGN_FAILED" rssi -> int?: return wifi_get_rssi_ resource_group_ + on_event_ state/int: + // TODO(kasper): We should be clearing the state in the + // $monitor.ResourceState_ object, but since we're only + // closing here it doesn't really matter. Room for + // improvement though. + if (state & (WIFI_DISCONNECTED | WIFI_IP_LOST)) != 0: + task:: close + // ---------------------------------------------------------------------------- wifi_init_: diff --git a/tests/negative/gold/illegal_system_call_test.gold b/tests/negative/gold/illegal_system_call_test.gold index 8b5837f98..e85359ead 100644 --- a/tests/negative/gold/illegal_system_call_test.gold +++ b/tests/negative/gold/illegal_system_call_test.gold @@ -1,7 +1,7 @@ As check failed: null is not a ServiceResource. 0: ServiceDefinition.resource /system/services.toit:155:5 1: ContainerServiceDefinition.handle <...>/system/containers.toit:129:19 - 2: ServiceManager_. /system/services.toit:329:15 + 2: ServiceManager_. /system/services.toit:332:15 3: RpcRequest_.process. /rpc/broker.toit:98:26 4: RpcRequest_.process /rpc/broker.toit:95:3 5: RpcRequestQueue_.ensure_processing_task_... /rpc/broker.toit:214:20