Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mavgen WLUA: Fix incorrect display of negative ints on mac #944

Merged
merged 1 commit into from
May 1, 2024

Conversation

shancock884
Copy link
Contributor

This PR is to fix mavlink/mavlink#1862, fix mavlink/mavlink#2073 and fix #701. (In particular mavlink/mavlink#2073 includes the discussion about how it was figured out!).

There is a bug in wireshark, whereby a double is cast to an unsigned int when it should be a signed int, before being used as a signed int. On Linux this works, but on Mac negative values get cut to zero (here is why).
Thanks to @rotu, a Wireshark fix has been merged in wireshark/wireshark#15438.

This PR provides a workaround/fix to avoid the issue completely, so that it will work even with older versions of Wireshark.

The fix is to not include the value argument when calling add_le function, and let Wireshark do the value extraction internally from the raw bytes. This avoids the code which does the bad casting. It also makes the generated lua file have about 20-25% fewer lines of code.

There are some case in which the value still needs to separately extracted:

  • The value argument is still required in the case when a float is used to represent an enum or bitmask, as on a number of command parameters.
  • Value is used when adding unit conversions to the displayed value.
  • Value is used for the bitmask extraction functions, as the input may have come from a command float parameter.

I've tested new LUA with some basic scenarios on Linux, and the CI also includes a decent selection of pytests for this.

@rotu

Fix is to not include the value argument when calling add_le function, and let Wireshark
do the value extraction internally. This avoids an issue where a double being cast to
a unsigned int (which should have been an int), becomes 0 on mac, but works on Linux.

The value argument is still required in the case when a float is used to represent an
enum or bitmask.
Also, value is used seperately when adding unit conversions and when doing any bitmask
extraction
@rotu
Copy link
Contributor

rotu commented Apr 30, 2024

lgtm!

@peterbarker peterbarker merged commit 052a201 into ArduPilot:master May 1, 2024
12 checks passed
@shancock884 shancock884 deleted the mac-neg-int-fix branch May 1, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants