Skip to content

Commit

Permalink
Isseu #614: Improve trimming
Browse files Browse the repository at this point in the history
  • Loading branch information
@gotthardp
gotthardp committed Mar 29, 2019
1 parent d41a724 commit 100715c
Showing 1 changed file with 20 additions and 2 deletions.
22 changes: 20 additions & 2 deletions src/lorawan_gw_forwarder.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ handle_info({udp, Socket, _Host, _Port, <<_Version, Token:16, 5, MAC:8/binary, D
error ->
{undefined, Tokens}
end,
case string:strip(binary_to_list(Data)) of
[] ->
case trim_json(Data) of
<<>> ->
% no error occured
ok;
_ ->
Expand Down Expand Up @@ -206,4 +206,22 @@ build_txpk(TxQ, RFch, Data) ->
lists:zip(record_info(fields, txq), tl(tuple_to_list(TxQ)))
).

% some gateways send <<0>>
trim_json(<<0, Rest/binary>>) ->
trim_json(Rest);
trim_json(<<$\s, Rest/binary>>) ->
trim_json(Rest);
trim_json(<<$\t, Rest/binary>>) ->
trim_json(Rest);
trim_json(Rest) ->
Rest.

-include_lib("eunit/include/eunit.hrl").

trim_test_() ->
[?_assertEqual(<<>>, trim_json(<<>>)),
?_assertEqual(<<>>, trim_json(<<0>>)),
?_assertEqual(<<>>, trim_json(<<" \t\t">>)),
?_assertEqual(<<"{\"one\": 1}">>, trim_json(<<" {\"one\": 1}">>))].

% end of file

4 comments on commit 100715c

@xypron
Copy link
Contributor

@xypron xypron commented on 100715c Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks strange to me:

 trim_json(<<0, Rest/binary>>) ->

As a C programmer I would expect a \0 at the end of a string not at the beginning.

@altishchenko
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xypron Looks ok to me. Will silently crop all zeros, tabs and spaces off the remaining Data, making sure that if anything 'non-space' is left there it will be returned to the case statement. In our case it should return only <<>>.

@xypron
Copy link
Contributor

@xypron xypron commented on 100715c Apr 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is an example of the trim function failing:

1> c(test).
{ok,test}
2> test:trim_json(<< 65, 66, 00 >>).
<<65,66,0>>

It simply does not remove trailing zeros. A C program will not produce a \0 byte at the beginning but at the end of a string. So I do not understand the rationale of Petr's patch.

The Kerlink gateway only produces a single 0x00 in the TX_ACK message, e.g.:

0x0000:  0257 e014 b1ee 0234 ddb9 1822 0800 4500
0x0010:  0029 fd7b 4000 3411 3705 c0a8 0116 c0a8
0x0020:  014f dba8 0690 0015 0cfc 02b5 4705 7276
0x0030:  ff00 13a4 0afa 00

@gotthardp
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function is meant to remove whitespace from a whitespace-only string. If there is a space, zero, two zeros, whatever, this fun reduces it to <<>>. If there is anything else than whitespace, then the standard JSON parse is invoked (on the non-trimmed string). Therefore, removing trailing zeros is superfluous.

Please sign in to comment.