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 multi-serial CRC error crash #21249

Merged

Conversation

rhapsodyv
Copy link
Member

@rhapsodyv rhapsodyv commented Mar 4, 2021

Description

If we get a CRC error when using more than one serial, the old code inside flush_and_request_resend will get the wrong serial index from ring_buffer.command_port(), because the current command wasn't queue yet.
So I made it receive the index by parameter.

I was having another crash too, related with the parameter const serial_index_t serial_ind. For some strange reason, the compiler was optimising the array access serial_state[serial_ind].last_N using a non sense value for serial_ind. The issue vanished when I started to use reference as parameter.
Link the pictures showing the issue: #21010 (comment)

Maybe it was a issue trigged by too many debugging sessions... or by wrong gcc optimisation... I don't know. If don't have pictures, I would not believe.
After a full build, it seems solved the const/ref issue..

I'm attaching a simple script that I used to test the serial.

Benefits

More stable multi serial.
May fix #21010 and #21244

test-serial.py.txt

@rhapsodyv rhapsodyv force-pushed the fix-multi-serial-on-error branch from 4bfb5cd to 610bd83 Compare March 4, 2021 15:24
@rhapsodyv
Copy link
Member Author

Bugfix is broken in more than one place...

@rhapsodyv
Copy link
Member Author

It seams pretty stable, running on two different serial ports, simulating 10% of bad crc commands:
multi-serial

@thinkyhead thinkyhead changed the title Fix crash on multi serial when a CRC error happen Fix multi-serial CRC error crash Mar 4, 2021
@thinkyhead thinkyhead merged commit 101f09a into MarlinFirmware:bugfix-2.0.x Mar 4, 2021
rmpel added a commit to rmpel/Marlin that referenced this pull request Mar 5, 2021
* bugfix-2.0.x: (135 commits)
  Tweak tests, consolidate pins target validation (MarlinFirmware#21254)
  [cron] Bump distribution date (2021-03-05)
  Fix multi-serial CRC error crash (MarlinFirmware#21249)
  Followup to MP_SCARA/TPARA patches (MarlinFirmware#21248)
  Remove extra G42
  Correct fan pins for MKS Robin Nano v3 (MarlinFirmware#21238)
  SMUFF => SMuFF (MarlinFirmware#21243)
  Implement G42, after all
  MK2_MULTIPLEXER dependency
  Update some py scripts
  Parking Extruder solenoid fix/cleanup
  Fix teensy35 tests
  [cron] Bump distribution date (2021-03-04)
  TPARA followup
  TPARA - 3DOF robot arm IK (MarlinFirmware#21005)
  misc. cleanup
  Improve opt_set (etc.) used for tests
  Fix MKS H43 compile (MarlinFirmware#21240)
  [cron] Bump distribution date (2021-03-03)
  Trust XY after Quiet Probing short sleep (MarlinFirmware#21237)
  ...

# Conflicts:
#	Marlin/Configuration.h
#	Marlin/Configuration_adv.h
rmpel added a commit to rmpel/Marlin that referenced this pull request Mar 6, 2021
* bugfix-2.0.x: (135 commits)
  Tweak tests, consolidate pins target validation (MarlinFirmware#21254)
  [cron] Bump distribution date (2021-03-05)
  Fix multi-serial CRC error crash (MarlinFirmware#21249)
  Followup to MP_SCARA/TPARA patches (MarlinFirmware#21248)
  Remove extra G42
  Correct fan pins for MKS Robin Nano v3 (MarlinFirmware#21238)
  SMUFF => SMuFF (MarlinFirmware#21243)
  Implement G42, after all
  MK2_MULTIPLEXER dependency
  Update some py scripts
  Parking Extruder solenoid fix/cleanup
  Fix teensy35 tests
  [cron] Bump distribution date (2021-03-04)
  TPARA followup
  TPARA - 3DOF robot arm IK (MarlinFirmware#21005)
  misc. cleanup
  Improve opt_set (etc.) used for tests
  Fix MKS H43 compile (MarlinFirmware#21240)
  [cron] Bump distribution date (2021-03-03)
  Trust XY after Quiet Probing short sleep (MarlinFirmware#21237)
  ...

# Conflicts:
#	Marlin/Configuration.h
#	Marlin/Configuration_adv.h
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
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
@rhapsodyv rhapsodyv deleted the fix-multi-serial-on-error branch May 30, 2021 22:56
rmpel added a commit to rmpel/Marlin that referenced this pull request Feb 3, 2022
* bugfix-2.0.x: (135 commits)
  Tweak tests, consolidate pins target validation (MarlinFirmware#21254)
  [cron] Bump distribution date (2021-03-05)
  Fix multi-serial CRC error crash (MarlinFirmware#21249)
  Followup to MP_SCARA/TPARA patches (MarlinFirmware#21248)
  Remove extra G42
  Correct fan pins for MKS Robin Nano v3 (MarlinFirmware#21238)
  SMUFF => SMuFF (MarlinFirmware#21243)
  Implement G42, after all
  MK2_MULTIPLEXER dependency
  Update some py scripts
  Parking Extruder solenoid fix/cleanup
  Fix teensy35 tests
  [cron] Bump distribution date (2021-03-04)
  TPARA followup
  TPARA - 3DOF robot arm IK (MarlinFirmware#21005)
  misc. cleanup
  Improve opt_set (etc.) used for tests
  Fix MKS H43 compile (MarlinFirmware#21240)
  [cron] Bump distribution date (2021-03-03)
  Trust XY after Quiet Probing short sleep (MarlinFirmware#21237)
  ...

# Conflicts:
#	Marlin/Configuration.h
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.

2 participants