Clean up obsolete code in Ruida driver #217

TheAssassin · 2024-03-22T02:42:59Z

First PR originating from #216. Cleans up obsolete properties within the code.

mgmax · 2024-03-23T21:42:32Z

The @Deprecated private transient properties must remain so that XStream doesn't error when loading the XML files from old versions.
(edit: only the field variable must be preserved; you can remove the method)

(I fear this knowledge is currently documented nowhere and I don't really know where to put it. The closest place is https://github.com/t-oster/VisiCut/wiki/Developing-a-new-Lasercutter-Driver , what do you think?)

The tests currently fail and claim that the driver output changed. Could you please double-check?

TheAssassin · 2024-03-23T21:44:40Z

I saw the failing test already. I probably forgot to re-run the tests. Too many open branches...

The wiki is probably the place for this information, but this goes under "general development remarks".

I'd like to suggest disabling this repository's wiki (or at least limiting the ability to edit to collaborators).

mgmax · 2024-03-24T17:21:34Z

The failing tests are probably because getRasterPadding is used by convertRasterizableFromVectorPart (but from superclass) but the default value is different in the superclass vs. here?

LibLaserCut/src/main/java/de/thomas_oster/liblasercut/LaserCutter.java

Lines 314 to 316 in e4f91f5

    
           public double getRasterPadding() { 
        
             return 5; 
        
           }

LibLaserCut/src/main/java/de/thomas_oster/liblasercut/LaserCutter.java

Line 373 in e4f91f5

    
           int overscan = Math.round((float)Util.mm2px(this.getRasterPadding() * (rp.cutDirectionleftToRight ? 1 : -1), resolution));

(Please double-check what's the intended padding value. I vaguely remember that some special command is used where the machine does the padding stuff on its own and therefore zero padding is fine.)

Furthermore, you added one accidental unneeded file.

mgmax · 2024-04-09T19:26:06Z

Marking as draft - Please mark as ready once the reported problems are fixed

TheAssassin · 2024-04-13T00:39:13Z

I had a look at the diff between the old and the new file. The changes (see below) do not make sense to me. The failure is in the removal of the properties in commit 895defc. Note: it is not the change in the call to convertRasterizableToVectorPart, it must be related to the removal of the properties.

@mgmax do you have any idea why that is?

--- de.thomas_oster.liblasercut.drivers.Ruida.out.dec	2024-04-13 02:28:03.253264492 +0200
+++ de.thomas_oster.liblasercut.drivers.Ruida.out.new.dec	2024-04-13 02:27:53.184988893 +0200
@@ -39,10 +39,10 @@
 Cut_Rel 0.051mm 5.08mm					a9 00 33 27 58 
 Cut_Abs 76.199mm 45.72mm				a8 00 00 04 53 27 00 00 02 65 18 
 Cut_Abs 101.6mm 50.8mm					a8 00 00 06 19 60 00 00 03 0c 70 
-Layer_Top_Left_E7_52 Layer:1 0.66mm 1.879mm		e7 52 01 00 00 00 05 14 00 00 00 0e 57 
-Layer_Bottom_Right_E7_53 Layer:1 1.117mm 2.133mm	e7 53 01 00 00 00 08 5d 00 00 00 10 55 
-Layer_Top_Left_E7_61 Layer:1 0.66mm 1.879mm		e7 61 01 00 00 00 05 14 00 00 00 0e 57 
-Layer_Bottom_Right_E7_62 Layer:1 1.117mm 2.133mm	e7 62 01 00 00 00 08 5d 00 00 00 10 55 
+Layer_Top_Left_E7_52 Layer:1 0.0mm 1.879mm		e7 52 01 00 00 00 00 00 00 00 00 0e 57 
+Layer_Bottom_Right_E7_53 Layer:1 6.095mm 2.133mm	e7 53 01 00 00 00 2f 4f 00 00 00 10 55 
+Layer_Top_Left_E7_61 Layer:1 0.0mm 1.879mm		e7 61 01 00 00 00 00 00 00 00 00 0e 57 
+Layer_Bottom_Right_E7_62 Layer:1 6.095mm 2.133mm	e7 62 01 00 00 00 2f 4f 00 00 00 10 55 
 Laser_1_Min_Pow_C6_31 Layer:1 100%			c6 31 01 7f 7f 
 Laser_1_Max_Pow_C6_32 Layer:1 100%			c6 32 01 7f 7f 
 Layer_Speed Layer:1 1000.0mm/s				c9 04 01 00 00 3d 04 40 
@@ -57,32 +57,44 @@
 Laser_1_Max_Pow_C6_02 100%				c6 02 7f 7f 
 CA_03 0f						ca 03 0f 
 CA 10 00						ca 10 00 
+Mov_Abs 0.0mm 1.879mm					88 00 00 00 00 00 00 00 00 0e 57 
 Mov_Abs 0.66mm 1.879mm					88 00 00 00 05 14 00 00 00 0e 57 
 Cut_Abs 0.761mm 1.879mm					a8 00 00 00 05 79 00 00 00 0e 57 
 Cut_Abs 0.863mm 1.879mm					a8 00 00 00 06 5f 00 00 00 0e 57 
 Cut_Abs 0.914mm 1.879mm					a8 00 00 00 07 12 00 00 00 0e 57 
 Cut_Abs 1.066mm 1.879mm					a8 00 00 00 08 2a 00 00 00 0e 57 
 Cut_Abs 1.117mm 1.879mm					a8 00 00 00 08 5d 00 00 00 0e 57 
+Mov_Abs 6.095mm 1.879mm					88 00 00 00 2f 4f 00 00 00 0e 57 
+Mov_Abs 6.095mm 1.93mm					88 00 00 00 2f 4f 00 00 00 0f 0a 
 Mov_Abs 1.117mm 1.93mm					88 00 00 00 08 5d 00 00 00 0f 0a 
 Cut_Abs 1.066mm 1.93mm					a8 00 00 00 08 2a 00 00 00 0f 0a 
 Cut_Abs 0.965mm 1.93mm					a8 00 00 00 07 45 00 00 00 0f 0a 
 Cut_Abs 0.863mm 1.93mm					a8 00 00 00 06 5f 00 00 00 0f 0a 
 Cut_Abs 0.812mm 1.93mm					a8 00 00 00 06 2c 00 00 00 0f 0a 
 Cut_Abs 0.66mm 1.93mm					a8 00 00 00 05 14 00 00 00 0f 0a 
+Mov_Abs 0.0mm 1.93mm					88 00 00 00 00 00 00 00 00 0f 0a 
+Mov_Abs 0.0mm 1.981mm					88 00 00 00 00 00 00 00 00 0f 3d 
 Mov_Abs 0.66mm 1.981mm					88 00 00 00 05 14 00 00 00 0f 3d 
 Cut_Abs 1.117mm 1.981mm					a8 00 00 00 08 5d 00 00 00 0f 3d 
+Mov_Abs 6.095mm 1.981mm					88 00 00 00 2f 4f 00 00 00 0f 3d 
+Mov_Abs 6.095mm 2.032mm					88 00 00 00 2f 4f 00 00 00 0f 70 
 Mov_Abs 1.117mm 2.032mm					88 00 00 00 08 5d 00 00 00 0f 70 
 Cut_Abs 0.761mm 2.032mm					a8 00 00 00 05 79 00 00 00 0f 70 
 Cut_Abs 0.66mm 2.032mm					a8 00 00 00 05 14 00 00 00 0f 70 
+Mov_Abs 0.0mm 2.032mm					88 00 00 00 00 00 00 00 00 0f 70 
+Mov_Abs 0.0mm 2.082mm					88 00 00 00 00 00 00 00 00 10 22 
 Mov_Abs 0.66mm 2.082mm					88 00 00 00 05 14 00 00 00 10 22 
 Cut_Abs 1.117mm 2.082mm					a8 00 00 00 08 5d 00 00 00 10 22 
+Mov_Abs 6.095mm 2.082mm					88 00 00 00 2f 4f 00 00 00 10 22 
+Mov_Abs 6.095mm 2.133mm					88 00 00 00 2f 4f 00 00 00 10 55 
 Mov_Abs 1.117mm 2.133mm					88 00 00 00 08 5d 00 00 00 10 55 
 Cut_Abs 0.761mm 2.133mm					a8 00 00 00 05 79 00 00 00 10 55 
 Cut_Abs 0.66mm 2.133mm					a8 00 00 00 05 14 00 00 00 10 55 
-Layer_Top_Left_E7_52 Layer:2 2.133mm 3.911mm		e7 52 02 00 00 00 10 55 00 00 00 1e 47 
-Layer_Bottom_Right_E7_53 Layer:2 2.59mm 4.165mm		e7 53 02 00 00 00 14 1e 00 00 00 20 45 
-Layer_Top_Left_E7_61 Layer:2 2.133mm 3.911mm		e7 61 02 00 00 00 10 55 00 00 00 1e 47 
-Layer_Bottom_Right_E7_62 Layer:2 2.59mm 4.165mm		e7 62 02 00 00 00 14 1e 00 00 00 20 45 
+Mov_Abs 0.0mm 2.133mm					88 00 00 00 00 00 00 00 00 10 55 
+Layer_Top_Left_E7_52 Layer:2 0.0mm 3.911mm		e7 52 02 00 00 00 00 00 00 00 00 1e 47 
+Layer_Bottom_Right_E7_53 Layer:2 7.569mm 4.165mm	e7 53 02 00 00 00 3b 11 00 00 00 20 45 
+Layer_Top_Left_E7_61 Layer:2 0.0mm 3.911mm		e7 61 02 00 00 00 00 00 00 00 00 1e 47 
+Layer_Bottom_Right_E7_62 Layer:2 7.569mm 4.165mm	e7 62 02 00 00 00 3b 11 00 00 00 20 45 
 Laser_1_Min_Pow_C6_31 Layer:2 100%			c6 31 02 7f 7f 
 Laser_1_Max_Pow_C6_32 Layer:2 100%			c6 32 02 7f 7f 
 Layer_Speed Layer:2 1000.0mm/s				c9 04 02 00 00 3d 04 40 
@@ -97,25 +109,35 @@
 Laser_1_Max_Pow_C6_02 100%				c6 02 7f 7f 
 CA_03 0f						ca 03 0f 
 CA 10 00						ca 10 00 
+Mov_Abs 0.0mm 3.911mm					88 00 00 00 00 00 00 00 00 1e 47 
 Mov_Abs 2.235mm 3.911mm					88 00 00 00 11 3b 00 00 00 1e 47 
 Cut_Abs 2.336mm 3.911mm					a8 00 00 00 12 20 00 00 00 1e 47 
 Mov_Abs 2.387mm 3.911mm					88 00 00 00 12 53 00 00 00 1e 47 
 Cut_Abs 2.54mm 3.911mm					a8 00 00 00 13 6c 00 00 00 1e 47 
+Mov_Abs 7.518mm 3.911mm					88 00 00 00 3a 5e 00 00 00 1e 47 
+Mov_Abs 7.518mm 3.962mm					88 00 00 00 3a 5e 00 00 00 1e 7a 
 Mov_Abs 2.54mm 3.962mm					88 00 00 00 13 6c 00 00 00 1e 7a 
 Cut_Abs 2.438mm 3.962mm					a8 00 00 00 13 06 00 00 00 1e 7a 
 Mov_Abs 2.336mm 3.962mm					88 00 00 00 12 20 00 00 00 1e 7a 
 Cut_Abs 2.285mm 3.962mm					a8 00 00 00 11 6d 00 00 00 1e 7a 
+Mov_Abs 0.0mm 3.962mm					88 00 00 00 00 00 00 00 00 1e 7a 
+Mov_Abs 0.0mm 4.013mm					88 00 00 00 00 00 00 00 00 1f 2d 
 Mov_Abs 2.133mm 4.013mm					88 00 00 00 10 55 00 00 00 1f 2d 
 Laser_1_Max_Pow_C6_02 75%				c6 02 5f 7f 
 Cut_Abs 2.184mm 4.013mm					a8 00 00 00 11 08 00 00 00 1f 2d 
 Mov_Abs 2.235mm 4.013mm					88 00 00 00 11 3b 00 00 00 1f 2d 
 Cut_Abs 2.59mm 4.013mm					a8 00 00 00 14 1e 00 00 00 1f 2d 
+Mov_Abs 7.569mm 4.013mm					88 00 00 00 3b 11 00 00 00 1f 2d 
+Mov_Abs 7.569mm 4.064mm					88 00 00 00 3b 11 00 00 00 1f 60 
 Mov_Abs 2.59mm 4.064mm					88 00 00 00 14 1e 00 00 00 1f 60 
 Laser_1_Max_Pow_C6_02 50%				c6 02 40 00 
 Cut_Abs 2.235mm 4.064mm					a8 00 00 00 11 3b 00 00 00 1f 60 
+Mov_Abs 0.0mm 4.064mm					88 00 00 00 00 00 00 00 00 1f 60 
+Mov_Abs 0.0mm 4.165mm					88 00 00 00 00 00 00 00 00 20 45 
 Mov_Abs 2.133mm 4.165mm					88 00 00 00 10 55 00 00 00 20 45 
 Laser_1_Max_Pow_C6_02 100%				c6 02 7f 7f 
 Cut_Abs 2.235mm 4.165mm					a8 00 00 00 11 3b 00 00 00 20 45 
-Work_Interval 01 06 20 365.0mm 365.0mm			da 01 06 20 00 00 00 02 6d 00 00 00 02 6d 
+Mov_Abs 7.213mm 4.165mm					88 00 00 00 38 2d 00 00 00 20 45 
+Work_Interval 01 06 20 418.0mm 418.0mm			da 01 06 20 00 00 00 03 22 00 00 00 03 22 
 Stop							e7 00 
 EOF							d7

mgmax · 2024-04-19T19:37:30Z

convertRasterizableToVectorPart is called correctly.
It uses the value of getRasterPadding of the laser device.
Before, it gets Ruida.getRasterPadding()=0.
Now, that method is removed, and therefore we "fall back" to LaserCutter.getRasterPadding()=5. Therefore the different behavior.

TheAssassin · 2024-04-19T19:40:40Z

The joy of uncommented code... I'll clean it up, thanks for your investigation.

mgmax

Thanks for the cleanup. Please have a look at my comments.

mgmax · 2024-04-20T19:20:50Z

src/main/java/de/thomas_oster/liblasercut/laserscript/.gradle/4.4.1/fileHashes/fileHashes.lock

Looks like you accidentally added this file

src/main/java/de/thomas_oster/liblasercut/drivers/Ruida.java

These properties were overlooked during the refactoring in said commit.

TheAssassin · 2024-04-21T14:04:19Z

Works for me, I'll quickly rebase it.

TheAssassin force-pushed the improve-ruida branch from eb8de7c to 895defc Compare March 23, 2024 21:50

mgmax marked this pull request as draft April 9, 2024 19:25

TheAssassin force-pushed the improve-ruida branch from a62c2ef to 9b9ad21 Compare April 19, 2024 20:38

TheAssassin marked this pull request as ready for review April 19, 2024 20:42

mgmax requested changes Apr 20, 2024

View reviewed changes

Remove properties obsoleted by c2bee3a

8869625

These properties were overlooked during the refactoring in said commit.

TheAssassin force-pushed the improve-ruida branch from 9b9ad21 to f26c6c9 Compare April 20, 2024 20:34

Clean up another obsolete property

1afe209

TheAssassin force-pushed the improve-ruida branch from f0826a8 to 1afe209 Compare April 21, 2024 14:05

TheAssassin requested a review from mgmax April 21, 2024 14:05

mgmax merged commit 1de68f3 into t-oster:master Apr 21, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up obsolete code in Ruida driver #217

Clean up obsolete code in Ruida driver #217

TheAssassin commented Mar 22, 2024

mgmax commented Mar 23, 2024 •

edited

Loading

TheAssassin commented Mar 23, 2024

mgmax commented Mar 24, 2024

mgmax commented Apr 9, 2024

TheAssassin commented Apr 13, 2024 •

edited

Loading

mgmax commented Apr 19, 2024

TheAssassin commented Apr 19, 2024

mgmax left a comment

mgmax Apr 20, 2024

TheAssassin commented Apr 21, 2024

Clean up obsolete code in Ruida driver #217

Clean up obsolete code in Ruida driver #217

Conversation

TheAssassin commented Mar 22, 2024

mgmax commented Mar 23, 2024 • edited Loading

TheAssassin commented Mar 23, 2024

mgmax commented Mar 24, 2024

mgmax commented Apr 9, 2024

TheAssassin commented Apr 13, 2024 • edited Loading

mgmax commented Apr 19, 2024

TheAssassin commented Apr 19, 2024

mgmax left a comment

Choose a reason for hiding this comment

mgmax Apr 20, 2024

Choose a reason for hiding this comment

TheAssassin commented Apr 21, 2024

mgmax commented Mar 23, 2024 •

edited

Loading

TheAssassin commented Apr 13, 2024 •

edited

Loading