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

Fix an issue, in which overlapping parameters were shown separately #228

Merged

Conversation

floroks
Copy link
Contributor

@floroks floroks commented Oct 21, 2023

Fix an issue, in which overlapping parameters were shown separately
Overlapping parameters happen when you define parameters with different bit-lengths at the same byte-position.

Example:
Define parameter X at position 3 with bit-length 24
Define parameter Y at position 3 with bit-length 8
Define parameter Z at position 4 with bit-length 8
Define parameter A at position 5 with bit-length 8

@floroks floroks force-pushed the fix_overlapping_parameters_display branch 5 times, most recently from 05e3c83 to ed1cac0 Compare October 21, 2023 11:11
@@ -411,6 +422,17 @@ def param_sort_key(param: Parameter) -> Tuple[int, int]:
formatted_lines.append(next_line)
next_line = ""

if 0 < param_idx < len(params) - 1 and hasattr(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer range checks to be oldschool:

Suggested change
if 0 < param_idx < len(params) - 1 and hasattr(
if 0 < param_idx and param_idx < len(params) - 1 and hasattr(

(while a < x < b works with python it feels too magical to me because (a < x) < b will not work in all cases)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Overlapping parameters happen when you define parameters with different
bit-lengths at the same byte-position.
@floroks floroks force-pushed the fix_overlapping_parameters_display branch from ed1cac0 to f6bf2e9 Compare October 21, 2023 12:52
@andlaus
Copy link
Collaborator

andlaus commented Oct 23, 2023

ok, thanks; merging.

(note to myself: BasicStructure._message_format_lines() is way too long, too complicated and should probably not be part of the class in the first place.)

@andlaus andlaus merged commit 41605c8 into mercedes-benz:main Oct 23, 2023
6 checks passed
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.

2 participants