Skip to content

[fix] read and write unsigned bytes in SignalPdu#57

Merged
leif81 merged 3 commits intoopen-dis:masterfrom
ngjunsiang:bug-46
May 18, 2024
Merged

[fix] read and write unsigned bytes in SignalPdu#57
leif81 merged 3 commits intoopen-dis:masterfrom
ngjunsiang:bug-46

Conversation

@ngjunsiang
Copy link
Copy Markdown
Contributor

Fixes #46

@leif81
Copy link
Copy Markdown
Member

leif81 commented May 17, 2024

Thank-you @ngjunsiang !
Do you think the other 4 lines referenced in the issue still require a change too?

@ngjunsiang
Copy link
Copy Markdown
Contributor Author

Thank-you @ngjunsiang ! Do you think the other 4 lines referenced in the issue still require a change too?

I couldn't find the other lines, then suddenly remembered I can browse the repository at the point the comment was made 😅

From the comment, the unaddressed lines are:
3605,3609,3618,3623

Mapping them to the current commit:

3605:
https://github.com/ngjunsiang/open-dis-python/blob/473d63abdc00b05703e5aa3246a2eb6f75239c08/opendis/dis7.py#L3789-L3790

3609:
https://github.com/ngjunsiang/open-dis-python/blob/473d63abdc00b05703e5aa3246a2eb6f75239c08/opendis/dis7.py#L3793-L3794

3618:
https://github.com/ngjunsiang/open-dis-python/blob/473d63abdc00b05703e5aa3246a2eb6f75239c08/opendis/dis7.py#L3800-L3801

3623:
https://github.com/ngjunsiang/open-dis-python/blob/473d63abdc00b05703e5aa3246a2eb6f75239c08/opendis/dis7.py#L3805-L3806

I'll push another commit to address these.

fixing lines 3790, 3794, 3801, 3806
@leif81
Copy link
Copy Markdown
Member

leif81 commented May 18, 2024

Thanks @ngjunsiang

What do you think about the original comment about line 7141 and 7157?

@ngjunsiang
Copy link
Copy Markdown
Contributor Author

According to IEEE1278.1-2012, dataLength and samples should be 16-bit unsigned ints as well. I have pushed the changes.

At this rate I can probably do a search for signed reads/writes to check possible bugs 😂

@ngjunsiang
Copy link
Copy Markdown
Contributor Author

At this rate I can probably do a search for signed reads/writes to check possible bugs 😂

A quick search reveals most of the signed read/write methods of the input and output stream class are little-used; where they are used, it is primarily for padding.

@leif81
Copy link
Copy Markdown
Member

leif81 commented May 18, 2024

Ok then thank-you for checking. In that case I suggest we leave the scope of the changes as is. I'll do a quick review and then merge in a moment

@leif81 leif81 merged commit bfe83a5 into open-dis:master May 18, 2024
@ngjunsiang ngjunsiang deleted the bug-46 branch May 18, 2024 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

signed bytes in signal pdu

2 participants