From 53122ca8766888f59459ee40b92b98192a8602ab Mon Sep 17 00:00:00 2001 From: macpie Date: Wed, 11 Oct 2023 10:55:58 -0700 Subject: [PATCH] Fix for double confirmed downlink (#1012) --- src/device/router_device_worker.erl | 55 ++++++----------- test/router_downlink_SUITE.erl | 94 +++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 35 deletions(-) diff --git a/src/device/router_device_worker.erl b/src/device/router_device_worker.erl index f581a56b7..57a56d95b 100644 --- a/src/device/router_device_worker.erl +++ b/src/device/router_device_worker.erl @@ -1548,12 +1548,27 @@ validate_frame( FrameCache, OfferCache ) -> - <> = blockchain_helium_packet_v1:payload(Packet), DeviceFCnt = router_device:fcnt(Device0), + %% This only applies when it's the first packet we see + %% If frame countain ACK=1 we should clear message from queue and go on next + QueueDeviceUpdates = + case {ACK, router_device:queue(Device0)} of + %% Check if acknowledging confirmed downlink + {1, [#downlink{confirmed = true} | T]} -> + [{queue, T}, {fcntdown, router_device:fcntdown_next_val(Device0)}]; + {1, _} -> + lager:warning("got ack when no confirmed downlinks in queue"), + []; + _ -> + [] + end, + QueueUpdatedDevice = router_device:update(QueueDeviceUpdates, Device0), + case MType of MType when MType == ?CONFIRMED_UP orelse MType == ?UNCONFIRMED_UP -> FrameAck = router_utils:mtype_to_ack(MType), @@ -1576,7 +1591,7 @@ validate_frame( Packet, PubKeyBin, Region, - Device0, + QueueUpdatedDevice, OfferCache, false ); @@ -1625,7 +1640,7 @@ validate_frame( Packet, PubKeyBin, Region, - Device0, + QueueUpdatedDevice, OfferCache, false ) @@ -1703,22 +1718,7 @@ validate_frame_(PacketFCnt, Packet, PubKeyBin, HotspotRegion, Device0, OfferCach {region, Region}, {last_known_datarate, DRIdx} ], - %% If frame countain ACK=1 we should clear message from queue and go on next - QueueDeviceUpdates = - case {ACK, router_device:queue(Device0)} of - %% Check if acknowledging confirmed downlink - {1, [#downlink{confirmed = true} | T]} -> - [{queue, T}, {fcntdown, router_device:fcntdown_next_val(Device0)}]; - {1, _} -> - lager:warning("got ack when no confirmed downlinks in queue"), - []; - _ -> - [] - end, - Device1 = router_device:update( - QueueDeviceUpdates ++ BaseDeviceUpdates, - Device0 - ), + Device1 = router_device:update(BaseDeviceUpdates, Device0), Frame = #frame{ mtype = MType, devaddr = DevAddr, @@ -1769,22 +1769,7 @@ validate_frame_(PacketFCnt, Packet, PubKeyBin, HotspotRegion, Device0, OfferCach {region, Region}, {last_known_datarate, DRIdx} ], - %% If frame countain ACK=1 we should clear message from queue and go on next - QueueDeviceUpdates = - case {ACK, router_device:queue(Device0)} of - %% Check if acknowledging confirmed downlink - {1, [#downlink{confirmed = true} | T]} -> - [{queue, T}, {fcntdown, router_device:fcntdown_next_val(Device0)}]; - {1, _} -> - lager:warning("got ack when no confirmed downlinks in queue"), - []; - _ -> - [] - end, - Device1 = router_device:update( - QueueDeviceUpdates ++ BaseDeviceUpdates, - Device0 - ), + Device1 = router_device:update(BaseDeviceUpdates, Device0), Frame = #frame{ mtype = MType, devaddr = DevAddr, diff --git a/test/router_downlink_SUITE.erl b/test/router_downlink_SUITE.erl index 1658c4d43..9ccb6029c 100644 --- a/test/router_downlink_SUITE.erl +++ b/test/router_downlink_SUITE.erl @@ -10,6 +10,7 @@ ]). -export([ + the_k3nt_test/1, http_downlink_test/1, console_tool_downlink_test/1, console_tool_downlink_order_test/1, @@ -57,6 +58,7 @@ groups() -> all_tests() -> [ + the_k3nt_test, http_downlink_test, console_tool_downlink_test, console_tool_downlink_order_test, @@ -89,6 +91,98 @@ end_per_testcase(TestCase, Config) -> %% TEST CASES %%-------------------------------------------------------------------- +%% AKA double donwlink queued before join +the_k3nt_test(Config) -> + Payload1 = <<"httpdownlink1">>, + Message1 = #{payload_raw => base64:encode(Payload1), region => <<"US915">>}, + Payload2 = <<"httpdownlink2">>, + Message2 = #{payload_raw => base64:encode(Payload2), region => <<"US915">>}, + %% Sending debug event from websocket + WSPid = + receive + {websocket_init, P} -> P + after 2500 -> ct:fail(websocket_init_timeout) + end, + WSPid ! {downlink, Message1}, + WSPid ! {downlink, Message2}, + + ok = test_utils:wait_until(fun() -> + case router_device_cache:get(?CONSOLE_DEVICE_ID) of + {error, not_found} -> false; + {ok, Device} -> 2 == length(router_device:queue(Device)) + end + end), + + #{ + pubkey_bin := PubKeyBin, + stream := Stream, + hotspot_name := _HotspotName + } = test_utils:join_device(Config), + + %% Waiting for reply from router to hotspot + test_utils:wait_state_channel_message(1250), + + {ok, Device0} = router_device_cache:get(?CONSOLE_DEVICE_ID), + Stream ! + {send, + test_utils:frame_packet( + ?UNCONFIRMED_UP, + PubKeyBin, + router_device:nwk_s_key(Device0), + router_device:app_s_key(Device0), + 0 + )}, + + %% Waiting for donwlink message on the hotspot + {ok, _} = test_utils:wait_state_channel_message( + {false, 1, Payload1}, + Device0, + Payload1, + ?UNCONFIRMED_DOWN, + 1, + 0, + 1, + 0 + ), + + ok = test_utils:wait_until(fun() -> + case router_device_cache:get(?CONSOLE_DEVICE_ID) of + {error, not_found} -> false; + {ok, Device} -> 1 == length(router_device:queue(Device)) + end + end), + + Stream ! + {send, + test_utils:frame_packet( + ?UNCONFIRMED_UP, + PubKeyBin, + router_device:nwk_s_key(Device0), + router_device:app_s_key(Device0), + 1 + )}, + + %% Waiting for donwlink message on the hotspot + {ok, _} = test_utils:wait_state_channel_message( + {false, 1, Payload2}, + Device0, + Payload2, + ?UNCONFIRMED_DOWN, + 0, + 0, + 1, + 1 + ), + + ok = test_utils:wait_until(fun() -> + case router_device_cache:get(?CONSOLE_DEVICE_ID) of + {error, not_found} -> false; + {ok, Device} -> 0 == length(router_device:queue(Device)) + end + end), + + ok. + http_downlink_test(Config) -> Payload = <<"httpdownlink">>, Message = #{payload_raw => base64:encode(Payload)},