diff --git a/CHANGELOG.md b/CHANGELOG.md index 55ce8ef..67b0bad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## Unreleased ### Added - Make the HTTP client follow redirects when interacting with Pairing API +- Add `unset_property` function to unset a path on a property interface. ### Changed - Increase the max certificate chain length to 10. diff --git a/lib/astarte_device.ex b/lib/astarte_device.ex index 1bde95d..7a0ca4d 100644 --- a/lib/astarte_device.ex +++ b/lib/astarte_device.ex @@ -276,6 +276,22 @@ defmodule Astarte.Device do :gen_statem.call(pid, {:set_property, interface_name, path, value}) end + @doc """ + Unset a property value to Astarte. + + This call is blocking and waits for the message to be ACKed at the MQTT level. + """ + @spec unset_property( + pid :: pid(), + interface_name :: String.t(), + path :: String.t() + ) :: + :ok + | {:error, reason :: term()} + def unset_property(pid, interface_name, path) do + :gen_statem.call(pid, {:unset_property, interface_name, path}) + end + @doc """ Blocks until the device succesfully connects to the broker, then returns `:ok`. @@ -544,6 +560,20 @@ defmodule Astarte.Device do {:keep_state_and_data, actions} end + def connected({:call, from}, {:unset_property, interface_name, path}, data) do + publish_params = [ + publish_type: :properties, + interface_name: interface_name, + path: path, + value: nil, + opts: [qos: 2] + ] + + reply = Impl.publish(publish_params, data) + actions = [{:reply, from, reply}] + {:keep_state_and_data, actions} + end + def connected(_event_type, _event, _data) do :keep_state_and_data end diff --git a/lib/astarte_device/impl.ex b/lib/astarte_device/impl.ex index 7924b37..564fd66 100644 --- a/lib/astarte_device/impl.ex +++ b/lib/astarte_device/impl.ex @@ -263,8 +263,7 @@ defmodule Astarte.Device.Impl do :ok <- validate_publish_type(publish_type, interface), :ok <- validate_ownership(:device, interface), :ok <- validate_path_and_type(interface, path, value), - payload_map = build_payload_map(value, opts), - {:ok, bson_payload} <- Cyanide.encode(payload_map) do + {:ok, payload} <- build_payload(value, opts) do publish_opts = Keyword.take(opts, [:qos]) "/" <> bare_path = path @@ -276,7 +275,7 @@ defmodule Astarte.Device.Impl do Enum.join([client_id, interface_name, bare_path], "/") end - @connection.publish_sync(client_id, topic, bson_payload, publish_opts) + @connection.publish_sync(client_id, topic, payload, publish_opts) end end @@ -452,6 +451,16 @@ defmodule Astarte.Device.Impl do end end + defp build_payload(nil, _opts) do + # If value is nil (which can only happen for an unset), we send an empty binary + {:ok, <<>>} + end + + defp build_payload(value, opts) do + payload_map = build_payload_map(value, opts) + Cyanide.encode(payload_map) + end + defp validate_publish_type(publish_type, %Interface{type: publish_type}) do :ok end @@ -477,10 +486,13 @@ defmodule Astarte.Device.Impl do end defp validate_path_and_type(%Interface{aggregation: :individual} = interface, path, value) do - mappings = interface.mappings + %Interface{ + mappings: mappings, + type: type + } = interface - with {:ok, %Mapping{value_type: expected_type}} <- find_mapping(path, mappings) do - Mapping.ValueType.validate_value(expected_type, value) + with {:ok, mapping} <- find_mapping(path, mappings) do + validate_value(type, mapping, value) end end @@ -494,6 +506,19 @@ defmodule Astarte.Device.Impl do {:error, :cannot_resolve_path} end + defp validate_value(:properties, %Mapping{allow_unset: allow_unset}, nil) do + # If value is nil on a properties interface, check if we can unset + if allow_unset do + :ok + else + {:error, :unset_not_allowed} + end + end + + defp validate_value(_type, %Mapping{value_type: expected_type}, value) do + Mapping.ValueType.validate_value(expected_type, value) + end + defp classify_error({:api, {:error, reason}}, log_tag) when reason in [:econnrefused, :closed, :timeout] do # Temporary errors diff --git a/test/astarte_device_test.exs b/test/astarte_device_test.exs index 44c9bb7..4258ba8 100644 --- a/test/astarte_device_test.exs +++ b/test/astarte_device_test.exs @@ -237,6 +237,48 @@ defmodule Astarte.DeviceTest do end end + describe "unset_property" do + setup [:set_mox_from_context, :verify_on_exit!, :init_device] + + test "fails on a datastream interface", %{device: device} do + interface = "org.astarteplatform.test.DeviceDatastream" + path = "/realValue" + value = 42.0 + + :ok = wait_for_connection() + + assert Device.unset_property(device, interface, path) == {:error, :datastream_interface} + end + + test "fails on a property interface without allow_unset", %{device: device} do + interface = "org.astarteplatform.test.DeviceProperties" + path = "/intValue" + value = 42 + + :ok = wait_for_connection() + + assert Device.unset_property(device, interface, path) == {:error, :unset_not_allowed} + end + + test "succeeds on a property interface", %{device: device} do + interface = "org.astarteplatform.test.DeviceProperties" + path = "/stringValue" + value = 42 + + full_path = "#{@realm}/#{@device_id}/#{interface}#{path}" + + ConnectionMock + |> expect(:publish_sync, fn _client_id, ^full_path, payload, [qos: 2] -> + assert payload == <<>> + :ok + end) + + :ok = wait_for_connection() + + assert Device.unset_property(device, interface, path) == :ok + end + end + defp init_device(_) do test_process = self() introspection_topic = @client_id diff --git a/test/interfaces/org.astarteplatform.DeviceProperties.json b/test/interfaces/org.astarteplatform.DeviceProperties.json index 5bc60b2..117b712 100644 --- a/test/interfaces/org.astarteplatform.DeviceProperties.json +++ b/test/interfaces/org.astarteplatform.DeviceProperties.json @@ -8,6 +8,11 @@ { "endpoint": "/intValue", "type": "integer" + }, + { + "endpoint": "/stringValue", + "type": "string", + "allow_unset": true } ] }