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

Nächster Versuch - V1.2 #3

Merged
merged 14 commits into from
Mar 7, 2020
Merged

Conversation

Bodengriller
Copy link
Collaborator

Ich habe meinen Fork nun nochmal komplett gelöscht und deine Repo neu geforkt.
...Keine Ahnung, wie es anders möglich gewesen wäre meinen Fork wieder auf den Stand deiner Repo zu bringen...

Z_SAFE_HOMING wurde wieder deaktiviert (Fehler in der V1.1 ... führt zu unerwünschten Ergebnissen -> siehe Kommentare)

Der Rest wurde so angepasst, wie besprochen.

Ich habe das Ganze nun einmal bei mir im Fork in den einzelnen Commits kommentiert -> das dürfte bei dir ziemlich unübersichtlich aussehen (Eine Liste von Commits mit einem hex-Link als Sprung zu der Stelle im Commit....sehr ungünstig gelöst.

Ich werde die Kommentare gleich einfach nochmal als neue Commits in diesem PullRequest anlegen, sobald er bei dir drüben ist - das sieht dann Übersichtlicher aus.

Hiermit solltest du keinen großartigen Konflikt bekommen.

Gib mal eine Rückmeldung, wie es ausschaut :)

Wie besprochen - ich hoffe ich habe nichts vergessen
ich hoffe ich habe auch hier nichts vergessen.
nicht mehr benötigt
Das ist nötig um einen Fehler beim Kompilieren zu unterbinden, der entsteht, wenn eine Probe aktiviert, Z_SAFE_HOMING aber deaktiviert ist.
Es ist gewollt, dass Babystepps speicherbar sind -> dafür muss der Z-Offset aktiviert werden -> dafür wiederum eine Probe.

Z_SAFE_HOMING bewirkt aber, dass man Z nur noch nach X und Y homen kann und dies irgendwo an einer bestimmten Position im Bett passiert.
Das ist sehr sinnvoll, wenn man einen Probe-Sensor montiert hat.
Wir wollen allerdings nur den Z-Offset missbrauchen, darum muss diese Sicherheitsfunktion weichen, weil sie bei uns nicht das gewünschte Ergebnis bringt.
Copy link
Collaborator Author

@Bodengriller Bodengriller left a comment

Choose a reason for hiding this comment

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

Mit diesem Review sollten die Kommentare dann schön im Quelltext zu sehen sein

Marlin/Configuration.h Show resolved Hide resolved
@@ -488,7 +488,8 @@
#define DEFAULT_Kp 10.85
#define DEFAULT_Ki 0.75
#define DEFAULT_Kd 39.12
// Sidewinder X1 bei 190° C

// Sidewinder X1 bei 190° C
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Leerzeile zur Übersicht

@@ -539,7 +540,6 @@
//#define PID_BED_DEBUG // Sends debug data to the serial port.

// Sidewinder X1 bei 60 Grad
// M304 P83.48 I8.15 D213.72
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unnötig

Copy link
Owner

Choose a reason for hiding this comment

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

Naja dann wären alle anderen Kommentare auch unnötig oder?
Man könnte noch dazuschreiben das es der GCode wäre um die Werte über die Konsole zu setzten.

#define DEFAULT_XJERK 8.0
#define DEFAULT_YJERK 8.0
#define DEFAULT_XJERK 10.0
#define DEFAULT_YJERK 10.0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unnötig, da junction deviation aktiv...dennoch als Gedankenstütze eingebracht -> Jerk 10 ist gut für SWX1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

muss noch kommentiert werden

@@ -812,7 +812,7 @@
* http://blog.kyneticcnc.com/2018/10/computing-junction-deviation-for-marlin.html
*/
#if DISABLED(CLASSIC_JERK)
#define JUNCTION_DEVIATION_MM 0.032 // (mm) Distance from real junction edge
#define JUNCTION_DEVIATION_MM 0.013 // (mm) Distance from real junction edge
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

0.013 hat sich scheinbar für den SWX1 etabliert!

@@ -878,7 +878,7 @@
* A Fix-Mounted Probe either doesn't deploy or needs manual deployment.
* (e.g., an inductive probe or a nozzle-based probe-switch.)
*/
//#define FIX_MOUNTED_PROBE // ASWX1-FW-MOD: Added For BABYSTEPPING
#define FIX_MOUNTED_PROBE // ASWX1-FW-MOD: activated to store BABYSTEPPING
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

muss aktiviert werden, damit man den Z-Offset zum Speichern der Babystepps nutzen kann


// Travel limits (mm) after homing, corresponding to endstop positions.
#define X_MIN_POS -2
#define X_MIN_POS 0
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Diese Einstellung wurde getestet und passt hervorragend!

Comment on lines +1370 to +1371
#define HOMING_FEEDRATE_XY (60*60)
#define HOMING_FEEDRATE_Z (10*60)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Langsameres Homing steigert die Wiederholgenauigkeit (Wichtig fürs Leveling!)

Copy link
Collaborator Author

@Bodengriller Bodengriller left a comment

Choose a reason for hiding this comment

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

Nächster Review, der alles deutlich übersichtlicher machen sollte, als die Einzel-Commits, die aus meinen Fork-Kommentaren erzeugt wurden

@@ -1464,7 +1464,7 @@
#if ENABLED(DOUBLECLICK_FOR_Z_BABYSTEPPING)
#define DOUBLECLICK_MAX_INTERVAL 1250 // Maximum interval between clicks, in milliseconds.
// Note: Extra time may be added to mitigate controller latency.
#define BABYSTEP_ALWAYS_AVAILABLE // Allow babystepping at all times (not just during movement).
//#define BABYSTEP_ALWAYS_AVAILABLE // Allow babystepping at all times (not just during movement).
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unnötig -> gilt nur für die LCD-Anzeige

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

wird rückgängig gemacht, um beim original zu bleiben

@@ -1473,7 +1473,7 @@

//#define BABYSTEP_DISPLAY_TOTAL // Display total babysteps since last G28

//#define BABYSTEP_ZPROBE_OFFSET // Combine M851 Z and Babystepping
#define BABYSTEP_ZPROBE_OFFSET // Combine M851 Z and Babystepping
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ist notwendig um die Babysteps in die Z-Offset-Variable zu schreiben, damit man diese mit M500 als Wert speichern kann.

Der Link zu 3D-Nexus hat sich scheinbar geändert - oder er war falsch.
Copy link
Owner

@pinguinpfleger pinguinpfleger left a comment

Choose a reason for hiding this comment

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

oh they url changed quickly, thx for fixing it

Copy link
Owner

@pinguinpfleger pinguinpfleger left a comment

Choose a reason for hiding this comment

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

Ja stimmt kann man weglassen.
Als Ausgangsmaterial wurden einfach alle Beispieldateien für den SX1 von Mariln kopiert.
Benutzt wird die Datei aber nicht ja.

@thierryzoller
Copy link

Ich habe meinen Fork nun nochmal komplett gelöscht und deine Repo neu geforkt.
...Keine Ahnung, wie es anders möglich gewesen wäre meinen Fork wieder auf den Stand deiner Repo zu bringen...

Geht ganz einfach wenn man den Kniff kennt

  1. Compare deinen Fork mit dem Upstream. Also Base du <-
  2. Dann rechts (Compare across Forks - WICHTIG)
  3. Dann rechts den Upstream
    ...

@Bodengriller
Copy link
Collaborator Author

Ich habe meinen Fork nun nochmal komplett gelöscht und deine Repo neu geforkt.
...Keine Ahnung, wie es anders möglich gewesen wäre meinen Fork wieder auf den Stand deiner Repo zu bringen...

Geht ganz einfach wenn man den Kniff kennt

  1. Compare deinen Fork mit dem Upstream. Also Base du <-
  2. Dann rechts (Compare across Forks - WICHTIG)
  3. Dann rechts den Upstream
    ...

Pinguinpfleger hatte es mir auch gerade erklärt - danke

@thierryzoller
Copy link

Wollte gerade Flashen :) Wann können wir mit dem Merge und Build rechnen ?

@Bodengriller
Copy link
Collaborator Author

Wir sind dran.
Ich hoffe, dass wir spätestens dieses WE fertig werden.
Bei mir läuft momentan diese Version problemfrei: https://github.com/Bodengriller/ASWX1-FW-MOD/releases/tag/ASWX1FW-MOD-v1.2
Falls da keine Interessenskonflikte mehr in der anstehenden Diskussion auftreten, wird es wohl darauf hinauslaufen.

Du kannst sie ja gerne mal flashen und auf Herz und Nieren testen, vielleicht fällt dir noch was auf.

Copy link
Owner

@pinguinpfleger pinguinpfleger left a comment

Choose a reason for hiding this comment

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

perfect

@pinguinpfleger pinguinpfleger merged commit 754f03f into pinguinpfleger:2.0.x Mar 7, 2020
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.

None yet

3 participants