Skip to content

Separate serial_index from serial's mask #21287

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

Merged
merged 13 commits into from
Mar 9, 2021

Conversation

X-Ryl669
Copy link
Contributor

@X-Ryl669 X-Ryl669 commented Mar 8, 2021

Description

There is a confusion in the code between serial_index_t and serial port masks. In some places (like the queue code), a serial_index_t of value -1 is used to mean no output. Yet a serial mask of 0xFF (which is -1) is used to mean output to all ports.
Since both were convertible, this leads to subtle bugs.

In this PR, I've created 2 different types: serial_index_t and SerialMask and there are not convertible. With this change, the compiler spotted confusing code (in autoreport where a mask was stored in an index variable, and in queue code) so I've fixed those at the same time.

Requirements

N/A

Benefits

Have the compiler does its job at spotting errors in the code instead of relying on the developer.

Configurations

N/A

Related Issues

N/A

Copy link
Contributor Author

@X-Ryl669 X-Ryl669 left a comment

Choose a reason for hiding this comment

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

Beware of range limits with within

@thinkyhead thinkyhead merged commit 55c31fb into MarlinFirmware:bugfix-2.0.x Mar 9, 2021
Vertabreak added a commit to Vertabreak/Marlin that referenced this pull request Mar 9, 2021
vyacheslav-shubin pushed a commit to vyacheslav-shubin/Marlin that referenced this pull request Mar 10, 2021
vyacheslav-shubin pushed a commit to vyacheslav-shubin/Marlin that referenced this pull request Mar 10, 2021
TyMi pushed a commit to TyMi/Marlin that referenced this pull request Mar 11, 2021
* bugfix-2.0.x: (248 commits)
  [cron] Bump distribution date (2021-03-11)
  Fix password menu stickiness before first auth (MarlinFirmware#21295)
  Lerdge-K TMC 2208/9 UART pins (MarlinFirmware#21299)
  Fix LERDGE 'extends' env references (MarlinFirmware#21305)
  Fix TouchMI stow in G34 (MarlinFirmware#21291)
  Fix MeatPack with per-serial-port instances (MarlinFirmware#21306)
  Tricked-out declaration
  Update MEATPACK test
  Number serial from 1 to match settings
  Clean up spaces and words
  Fix serial index types
  Add binary file transfer test
  fix meat pack internal buffer for multi serial
  [cron] Bump distribution date (2021-03-10)
  Fix LPC + TMC boot loop (MarlinFirmware#21298)
  Distinguish serial index from mask (MarlinFirmware#21287)
  Host Keepalive followup (MarlinFirmware#21290)
  [cron] Bump distribution date (2021-03-09)
  CUSTOM_USER_BUTTONS followup (MarlinFirmware#21284)
  Fix Host Keepalive serial target (MarlinFirmware#21283)
  ...

# Conflicts:
#	Marlin/Configuration.h
#	Marlin/Configuration_adv.h
chrisjenda pushed a commit to chrisjenda/Marlin that referenced this pull request Apr 3, 2021
W4tel-BiDi pushed a commit to W4tel-BiDi/Marlin that referenced this pull request Apr 5, 2021
thinkyhead pushed a commit that referenced this pull request Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants