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 line parsing #16840

Merged
merged 2 commits into from
Feb 14, 2020
Merged

Fix line parsing #16840

merged 2 commits into from
Feb 14, 2020

Conversation

GMagician
Copy link
Contributor

@GMagician GMagician commented Feb 11, 2020

Fix #16838

@GMagician GMagician changed the title Initializzation serial state using proper constant Fix serial communiction Feb 12, 2020
@GMagician GMagician changed the title Fix serial communiction Fix line parsing Feb 12, 2020
@GMagician
Copy link
Contributor Author

@thinkyhead is it expected that thermalManager.manage_heater is called every sent gcode line?
Since windows sends 'cr+lf', 'cr' handle message, 'lf' handle and empty message

@AnHardt
Copy link
Member

AnHardt commented Feb 13, 2020

thermalManager.manage_heater is called much more frequent than that, in the idle process - as long there is enough power left to idle. Don't care about that. thermalManager.manage_heater returns immediately if there is nothing to do, but we want it to be in time - if there is something to do. (If there is something to do is set in an interrupt - so can occur at any time.)

@GMagician
Copy link
Contributor Author

@AnHardt if it is already been called elsewhere (and often), why is necessary here?

@thinkyhead
Copy link
Member

@AnHardt if it is already been called elsewhere (and often), why is necessary here?

Please read #7449 where we fixed a watchdog reset issue. (Thanks again, GitLens!)

@thinkyhead thinkyhead merged commit 0e17d10 into MarlinFirmware:bugfix-2.0.x Feb 14, 2020
@AnHardt
Copy link
Member

AnHardt commented Feb 14, 2020

What we want is accepting lines with the endings: 'LF', 'CR', 'CR-LF' and 'LF-CR' calling thermalManager.manage_heater even with a empty line.
If you can find a faster or smaller way to handle the doubles without processing an additional empty line, tell us about that.

@GMagician
Copy link
Contributor Author

GMagician commented Feb 14, 2020

I'll check out code. An idea may be, if info is available, to call it when empty line and more than 75% of watchdog time has been passed before previous call, but if execution is not so a big deal (optimized actions) maybe this check may be worst

@GMagician GMagician deleted the typo-fix branch February 14, 2020 08:42
@AnHardt
Copy link
Member

AnHardt commented Feb 14, 2020

I thought more a bout something simple - not touching every HAL, like:
Finding the right place for something like:
If the current char is LR or CR peek the next char - if CR or LF, read an drop it.
Or avoiding peek:
If reading CR or LF (check a flag if already set (drop this char) else (set the flag)) else (erase the flag).

'thermalManager.manage_heater' gets new data to process about 5 times a second. We want to process the data without much jitter. Calling after 3 seconds is not acceptable. When the watchdog barks something went really wrong.

If i remember correctly data is ready after: 1/1024 seconds * number_of__ADCs * 2_phases * OVERSAMPLES

In theory we could call the PID-calculation in a irregular rhythm. But then we had to scale the float PID-constants by the real interval, not a constant. For the AVRs that would take forever - is not acceptable.

@thinkyhead
Copy link
Member

thinkyhead commented Feb 15, 2020

If i remember correctly data is ready after: 1/1024 seconds * number_of__ADCs * 2_phases * OVERSAMPLES

That's about right. The AVR version of Temperature::isr runs at "just under 1kHz" and the rest follow suit. There's a setup phase for each sensor followed by a read, followed by another setup phase…

It has occurred to me that the Setup phase for the next sensor could actually be done right after the Read phase of the previous sensor. So we could actually double the sensor read frequency with one small change.

…and if the different MCUs allow for multiple ADC conversions in parallel, or require shorter setup-to-read periods, then we should also take advantage of those features.

@thinkyhead
Copy link
Member

If the current char is LR or CR peek the next char - if CR or LF, read an drop it.

Another way to manage this is to limit to the number of empty lines that can be skipped and just return to the main loop without processing if that number is exceeded. This also ensures that other queue processing from other sources isn't blocked.

@AnHardt
Copy link
Member

AnHardt commented Feb 16, 2020

Another way to manage this is to limit to the number of empty lines that can be skipped and just return to the main loop without processing if that number is exceeded. This also ensures that other queue processing from other sources isn't blocked.

If placed before removing comments no 'long' blocking should occur. If there are rally many consecutive empty lines those are processed really fast - until the serial input buffer is empty - and we are idling again.

However this makes only sense when done faster than calling and returning a function without parameters plus one decision. With an intermediate 'always inline' function - only one decision. That will be hard to beat.

In our thermal systems we have death-times (times between switching a heat-element on and registering an effect at the thermometer) of several seconds. Caling PID more often than 4-5 times per second does not make much sense. Maybe, as long as an uint16_t summ (or 32bit for the 32bitters) is not flowing over, more over-samples could give useful less noise and/or a better resolution.

For the STM32s the ADC can be read by DMA and stored in a buffer. In that case always the newest values could be available without any delay. Actually we do use busy waiting analogeRead() where the two steps don't make any sense.

StJa added a commit to StJa/Marlin that referenced this pull request Mar 9, 2020
* Add sanity-check for new Advanced Pause option

Followup to MarlinFirmware#16372

* Include macros for delta ABC

* Update Russian language (MarlinFirmware#16745)

* Fix BTT SKR 1.4 extra endstop pins (MarlinFirmware#16738)

* Option for Trigorilla 1.4 with add-on endstops board (MarlinFirmware#16737)

* Consistent M112 with Emergency Parser (MarlinFirmware#16747)

* Improve mfadd helper script

- Use the original branch name if none is supplied
- Set the remote tracking to the source
- Accept User/Branch or User:Branch syntax

* Clean up i2c encoder, sanitize serial

* Misc cleanup, whitespace

* Encapsulate probe as singleton class (MarlinFirmware#16751)

* G34 automatic point assignment (MarlinFirmware#16473)

* Fix Temperature::over_autostart_threshold (MarlinFirmware#16749)

* Update Russian language (MarlinFirmware#16750)

* Fix CURRENT_STEP_DOWN compile error

* Drop obsolete SD special char handling

See MarlinFirmware#14035

* Probe singleton patch

Followup to MarlinFirmware#16751

* Fix RGB / Neopixel white color bug

See MarlinFirmware#16752

* Suppress a compile warning

* More 8-extruder fixups

* Add EXP labels to SKR pins

* Fix LPC build with USE_WATCHDOG off

* Minor string storage optimization

* Apply REPEAT, RREPEAT, and loop macros (MarlinFirmware#16757)

* Revert breaking change to _FAN_PWM macro

* Add Z_AFTER_HOMING to raise Z more in G28 (MarlinFirmware#16755)

* Corner Leveling: Add inset for each side (MarlinFirmware#16759)

* (c) 2020

* Tweak mfqp script

* Use a different Configurations branch for CI

* Fix LCD Z Move character LCD display line (MarlinFirmware#16772)

* Fix warning for ESP32 (MarlinFirmware#16771)

* [cron] Bump distribution date

* Force T0 in UBL G29 on all multi-hotend setups (MarlinFirmware#16774)

* Keep secure credentials in a separate config file (MarlinFirmware#16773)

* STM32duino - Use SDIO for onboard SD (MarlinFirmware#16756)

* Fix E stepper stays on bug

Fixes MarlinFirmware#16753

* Fix Arduino IDE compile for DUE

Fixes MarlinFirmware#16767

* Fix CALIBRATION_GCODE pin handling

* Upgrade an ifdef

* More updates for 8 extruders, REPEAT

* [cron] Bump distribution date (2020-02-05)

* Add MKS Base 1.6 board (MarlinFirmware#16783)

* Direct download link for configs

* [cron] Bump distribution date (2020-02-06)

* Split up MKS_RUMBA32 into two variants (MarlinFirmware#16781)

* G26: Allow to set retraction for UBL mesh test (MarlinFirmware#16511)

* Remove extraneous Serial init (MarlinFirmware#16794)

* Fix probe with multi-endstops (MarlinFirmware#16793)

* [cron] Bump distribution date (2020-02-07)

* [cron] Bump distribution date (2020-02-08)

* Clean up Makefle indentation

* Add .editorconfig file

* Tweak ABL logging, document probing

* [cron] Bump distribution date (2020-02-09)

* Coolstep for TMC2130, 2209, 5130, 5160 (MarlinFirmware#16790)

* Better probe fail handling (MarlinFirmware#16811)

* Adafruit Grand Central M4 fixes (MarlinFirmware#16812)

* Minor HAL cleanup

* Move MSG_MARLIN

* Show print time with PRINTER_EVENT_LEDS

* Tweak parser warning

* Bump config version to 020004 (MarlinFirmware#16816)

* Add PID, probe offsets to ExtUI (MarlinFirmware#16792)

* [cron] Bump distribution date (2020-02-10)

* Tweak LPC1768 upload py script

* Add mftest -b (auto-build) and -u (upload)

- Implement the equivalent of auto-build for the shell environment by using the MOTHERBOARD setting to look up the env: entries.

* Revert change to AXIS_DRIVER_TYPE_X2

- Revisit this to figure out why it breaks

* Revert "Coolstep for TMC2130, 2209, 5130, 5160"

Reverting MarlinFirmware#16790 as not ready for primetime.

* Add a caution to drivers.h

* Update MKS BASE and v1.6 pins (MarlinFirmware#16806)

* Add g-code quoted strings, improve stream code (MarlinFirmware#16818)

* Fix out-of-order M0 after SD printing

Fixes MarlinFirmware#14774

Co-Authored-By: tol2cj <tol2cj@users.noreply.github.com>

* Fix out-of-order M0 after SD printing

Fixes MarlinFirmware#14774

Co-Authored-By: tol2cj <tol2cj@users.noreply.github.com>

* Enable hotend / bed PID separately in ExtUI (MarlinFirmware#16827)

* Fix MKS Robin Nano platformio.ini entry (MarlinFirmware#16826)

* Unify step pulse timing of ISR / babystep (MarlinFirmware#16813)

* [cron] Bump distribution date (2020-02-11)

* Update SAMD51 EEPROM repo link (MarlinFirmware#16832)

* Undo driver type auto-assignment for now

Good general concept but needs more time to develop and group with a stepper suite.

* No Z sensorless req'd if homing with probe

Fixes MarlinFirmware#16674

* Recommend Z Safe Homing

Co-Authored-By: Vertabreaker <opyrus@hotmail.com>

* Use prior babystep delay method (MarlinFirmware#16833)

* Function-style critical section macros

* Fix up tests

* Simplify old safe homing sanity check

* Prevent pin glitches on out commutation (MarlinFirmware#16835)

Better for switching from pulled input to output and also set real output (with no input enabled).

* [cron] Bump distribution date (2020-02-12)

* Define MarlinSerial instances for DGUS (MarlinFirmware#16841)

* No limit needed on this raise

Remove an extraneous limit from MarlinFirmware#16811.

* [cron] Bump distribution date (2020-02-13)

* [cron] Bump distribution date (2020-02-14)

* Fix G-code line parsing (MarlinFirmware#16840)

* Ping the job timer in M140 (MarlinFirmware#16849)

* Remove unused queue.stopped_N (MarlinFirmware#16850)

* Don't assert safe homing for delta/scara

* Fix ESP32 warning, specify supported version

* Add ESPAsyncTCP to lib_ignore (MarlinFirmware#16844)

* Add RAMPS 1.4.4 to AGCM4 (MarlinFirmware#16606)

* Clean up host actions code (MarlinFirmware#16856)

* Optimize "Dismiss" string

* Clean up stepper and babystep (MarlinFirmware#16857)

* Fysetc S6 pins / LCD updates (MarlinFirmware#16830)

* [cron] Bump distribution date (2020-02-15)

* Fix mftest -b and -u. Add --help.

* Fix a BORG compile warning

* Fix byte-to-percent display

Fixes MarlinFirmware#16866

* Conceal float rounding errors on display

Fix MarlinFirmware#16866

* [cron] Bump distribution date (2020-02-16)

* Double ADC read frequency (MarlinFirmware#16864)

* EXPERIMENTAL integrated BABYSTEPPING (MarlinFirmware#16829)

* Show '*' for zero 'stst' flag

* Require TMCStepper 0.6.2

* Defer updated ADC

* Move SAMD51 Temperature timer to RTC (MarlinFirmware#16868)

* Fix unknown command on empty lines (MarlinFirmware#16867)

* Fix mftest -b -u line match

* Update French language (MarlinFirmware#16877)

* Put ESP32 I2S stepper task and Marlin on the same core (MarlinFirmware#16874)

* Fix babystep include, typos in stepper.cpp

Fix MarlinFirmware#16881

* [cron] Bump distribution date (2020-02-17)

* [cron] Bump distribution date (2020-02-18)

* [cron] Bump distribution date (2020-02-19)

* [cron] Bump distribution date (2020-02-20)

* Serial redirect for Move Command when stopping (MarlinFirmware#16906)

* [cron] Bump distribution date (2020-02-21)

* Single envs for specific boards

* Inline manage_inactivity, tweak autoreport_paused

* Function for CONFIG_ECHO_HEADING

* Show end prompt with Print Event LEDs

* Add a note on EEPROM todo

* Tweak process_line_done for speed

* More EEPROM cleanup

* Followup to autoreport patch (MarlinFirmware#16914)

See a1f026f

* Disable spreadcycle in tmc_enable_stallguard<2209> (MarlinFirmware#16890)

* Fix EEPROM errors with EXTRUDERS == 0 (MarlinFirmware#16898)

* Add PICA shields support (MarlinFirmware#16891)

* Tweak pins spacing, comments

* Version 2.0.4 Release

* [cron] Bump distribution date (2020-02-22)

* Use moves_free in ok_to_send

* Hotfix for Babystepping

* CoreXY Babystepping hotfix

* Use moves_free in ok_to_send

* [cron] Bump distribution date (2020-02-22)

* Hotfix for Babystepping

* [ESP32] Allow user to define pins for hardware Serial1 and Serial2 (MarlinFirmware#16918)

* CoreXY Babystepping hotfix

* Version 2.0.4.1 Release

* [cron] Bump distribution date (2020-02-23)

* Finish Custom User Menu sanity-check (MarlinFirmware#16917)

* Followup to babystep hotfix

* Fix M0/M1 broken wait loop (MarlinFirmware#16921)

* Define ANET_FULL_GRAPHICS_LCD pins for SKR 1.4 (MarlinFirmware#16928)

* Version 2.0.4.2 Release

* Suppress "packed member" warning

* Suppress "packed member" warning

* Commit last SD line before fileHasFinished

* Allow LCD_PIXEL_WIDTH/HEIGHT override

* Allow USE_GCODE_SUBCODES for debugging

* Sync Italian language (MarlinFirmware#16935)

* Reduce default TMC baudrate to 57600 with Software Serial (MarlinFirmware#16930)

* [cron] Bump distribution date (2020-02-24)

* Fix AXIS_HAS_SW_SERIAL

* Simplified E_AXIS_HAS macro

* "Init. Media" => "Attach Media"

* Fix Babystepping loop (again)

* BS_TOTAL_AXIS => BS_TOTAL_IND

* Allow Z_SAFE_HOMING_POINT outside bed (MarlinFirmware#16945)

* Restore tabs in Makefile (MarlinFirmware#16944)

* Fix card_eof error

* Version 2.0.4.3 Release

* Allow Z_SAFE_HOMING_POINT outside bed (MarlinFirmware#16945)

* Restore tabs in Makefile (MarlinFirmware#16944)

* Fix card_eof error

* [cron] Bump distribution date (2020-02-25)

* Update POWER_LOSS_PIN comment (MarlinFirmware#16957)

* Update Italian language (MarlinFirmware#16947)

* Fix LCD cutter/bed icons overlapping (MarlinFirmware#16956)

* Fix SKR 1.4 Turbo SD_DETECT_PIN (MarlinFirmware#16955)

* Fix the wait loop in M0 / M1

* [cron] Bump distribution date (2020-02-26)

* Ensure proper SD print completion (MarlinFirmware#16967)

* HAS_SDCARD_CONNECTION is more obsolete

* Fix GTR10 overlapping defines (MarlinFirmware#16976)

* Toolchange improvements (MarlinFirmware#16979)

* Language: "failsafe" => "Defaults"

* Use a STR_ prefix for non-translated strings

* Set LCD status for EEPROM errors (MarlinFirmware#16977)

* More serial strings

* Add LPC1768 Serial ports for pinsDebug (MarlinFirmware#16980)

* Correct SKR expansion port pins (MarlinFirmware#16974)

* String optimize followup

* Fix Trinamic pulse rate auto-assignment (MarlinFirmware#16966)

* Allow weird probe values in G33

* Serial strings in macros

* Sanity check for LPC serial pin conflict (MarlinFirmware#16981)

* Allow servo features in combination (MarlinFirmware#16960)

* Add TRAVEL_EXTRA_XYJERK option

See MarlinFirmware#16949

Co-Authored-By: josedpedroso <josedpedroso@users.noreply.github.com>

* Quick-homing sensorless back-off (MarlinFirmware#16872)

* Prevent park_point compile error

* More extra travel jerk changes

Co-Authored-By: josedpedroso <josedpedroso@users.noreply.github.com>

* Fix unified status bed temp display

* Allow print recovery after parking

* Case-insensitive g-code option (MarlinFirmware#16932)

* Define DIAG pins for MKS SGen-L

* Handle print completed LED event in M0

* [cron] Bump distribution date (2020-02-27)

* Fix planner.cpp compile (MarlinFirmware#16996)

* Update Slovak language (MarlinFirmware#17002)

* Version 2.0.4.4 Release

Co-authored-by: Scott Lahteine <github@thinkyhead.com>
Co-authored-by: Acenotass <44540957+Acenotass@users.noreply.github.com>
Co-authored-by: rebel1 <453277+rebel1@users.noreply.github.com>
Co-authored-by: Jason Smith <jason.inet@gmail.com>
Co-authored-by: Scott Lahteine <thinkyhead@users.noreply.github.com>
Co-authored-by: InsanityAutomation <38436470+InsanityAutomation@users.noreply.github.com>
Co-authored-by: ellensp <ellensp@hotmail.com>
Co-authored-by: felixstorm <felix.storm@glueckkanja.com>
Co-authored-by: Bob Kuhn <bob.kuhn@att.net>
Co-authored-by: thisiskeithb <13375512+thisiskeithb@users.noreply.github.com>
Co-authored-by: Tanguy Pruvot <tpruvot@users.noreply.github.com>
Co-authored-by: Robby Candra <robbycandra.mail@gmail.com>
Co-authored-by: Robert Stein <robert.stein@requisis.com>
Co-authored-by: Fabio Santos <fabiosan@live.com>
Co-authored-by: Giuliano Zaro <3684609+GMagician@users.noreply.github.com>
Co-authored-by: Daniel Mazurkiewicz <daniel@latanie.info>
Co-authored-by: tol2cj <tol2cj@users.noreply.github.com>
Co-authored-by: proferabg <proferabg@users.noreply.github.com>
Co-authored-by: darksiah <siatam@gmail.com>
Co-authored-by: Vertabreaker <opyrus@hotmail.com>
Co-authored-by: Gaston Dombiak <gdombiak@gmail.com>
Co-authored-by: vivian-ng <vivian@maplerain.com>
Co-authored-by: George Fu <nailao_5918@163.com>
Co-authored-by: Karl Andersson <karl@iaccess.se>
Co-authored-by: Jamie <vector76@users.noreply.github.com>
Co-authored-by: ZMiguel Alves <zmiguel@users.noreply.github.com>
Co-authored-by: Marcio T <mlt4356-github@yahoo.com>
Co-authored-by: josedpedroso <josedpedroso@users.noreply.github.com>
Co-authored-by: Makoto Schoppert <makotosan@gmail.com>
Co-authored-by: Roman Moravčík <roman.moravcik@gmail.com>
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.

3 participants