Hi, let's refactor

Software Aog issues (no bugs)
1

Hi, I’m Joost, I’m Dutch and live in the South of The Netherlands. Although AgOpenGPS is quite new to me, I like it a lot.
(For a variety of reasons.) As a retired software guy, I would like to contribute to your project.

When looking at the code, what strikes me most is that large parts of the code repeat similar calculations. Most of them involve 2D points
(currently of type vec2) and headings, (currently a double without any additional functionality.)

I would like to introduce three new structs, GeoCoord, GeoHeading, GeoDelta, that implement a few functions for operating overloading.
These new structs enable simplication of (almost) all calculations that involve 2D coords, 2D offsets, or headings.

For example the two following lines in the file CABLine.cs.

            if (mf.isReverse ? isHeadingSameWay : !isHeadingSameWay)
            {
                goalPointAB.easting = rEastAB - (Math.Sin(abHeading) * goalPointDistance);
                goalPointAB.northing = rNorthAB - (Math.Cos(abHeading) * goalPointDistance);
            }
            else
            {
                goalPointAB.easting = rEastAB + (Math.Sin(abHeading) * goalPointDistance);
                goalPointAB.northing = rNorthAB + (Math.Cos(abHeading) * goalPointDistance);
            }

Can be simplified to:

            if (mf.isReverse ? isHeadingSameWay : !isHeadingSameWay)
            {
                goalPointAB = rAB - abHeading * goalPointDistance;
            }
            else
            {
                goalPointAB = rAB + abHeading * goalPointDistance;
            }

Note that code in total has more than 200 lines using Sin() in a similar way, so large parts of the code can be simplified similarly.
Just to be clear, I also like to volunteer to rewrite and simplify all calculations that currently use the type vec2.

Ofcourse this refactoring step will only become a success if other people favor this idea too. So the question is:

  • what is your opinion?
  • what is the best way to get me started on this topic?

Best regards to all of you,
Joost

1 Like
2

Welcome to the community. You posted too soon after signing up, so post had to be approved manually.

I am not a programmer, but your suggestions look usable.

3

Sounds good to me, refactoring - especially for simplicity and clarity - is always a good thing.

Wondering if we could add any unit testing somehow ?

1 Like
5

Submit a pull request with your improved code. I am sure Brian and the other Dev’s would welcome the help.

1 Like
6

Definitely clone Brian’s AgOpenGPS repository. Perhaps you might want to touch base with him on which branch might be the most appropriate to work off of. You’ll also probably wish to be added to the development Telegram group.

In your software development work, did you use git? If not I highly recommend you learn git and become proficient with it because it is very powerful and enables the sort of refactoring to occur in a way that is possible to merge back into the original AOG tree at some point. You need at least a rudimentary understanding of git to work on AOG.

Although the AOG code base is large now, and complicated, the flow is understandable. I recommend you familiarize yourself with the main loop, which is driven by the GPS fixes.

I think your idea to abstract the various components needed by the different kinds of tracks is an excellent one. And code simplifications are always welcome. Last winter I managed to simply my main contribution to AOG from half a dozen lines of code originally down to just one line!

Also be aware that despite inaction for the last 6 months or more, there is a C++ Qt code base out there that made a lot of progress last winter. After I get over an illness I aim to work on it some more, to port the unified tracks code over from C#.

2 Likes
7

Hi Andy,

I do not see any unittest in the code so. Also I do not see any framework present for unittesting.
Anyway, as part of the changes, I will add a file Test_GeoBase.cs that includes test functions for
GeoCoord, GeoDelta and GeoHeading. Each function will return the number of bugs found, and hopefully, one day, if we introduce unit testing these function will be included.
The new files will be small and almost trivial so I expect no bugs there. The risc is that I introduce a
bug in one of the other (about) 65 files I want to change.

To minimize that risc, I will:

  1. Write the new code without removing the old code
  2. Add asserts to compare the new results against the old results.
  3. Test basic functionallity using the simulator and solve the bugs found.
  4. Test more and try to cover as much of the functionality
  5. Remove the old code

The problem for me is 4). Currently I only know a small part of the funtionality.

Best regards,
Joost

2 Likes
8

Hi Joost maybe you want to join the dutch telegram group for Dutch Aog users. We planning to do a winter meeting with some actual machines running aog and all. And share knowledge and ideas.

Groeten Raymond

3 Likes
9

Yes, thank you. I joined :wink:

10

Are you inviting international contributors? :stuck_out_tongue:

12

Your welcome to join mate anytime

1 Like
13

Greetings @Jozef!

How many new projects are you interested in? This summer I started using OpenGradeX, one of the few OpenGrade variants. The original was based on an old version of AgOpenGPS, then it was modified by @Pat to become OpenGrade3D and @BlackAce also modified a version called OpenGradeX. I started modifying OpenGradeX to work with wired UDP modules like the All-In-One autosteer PCB and make some bug fixes as it appears BlackAce has moved on to other projects and his wireless ESP32 module PCB design is not available. I feel the OpenGrade series of programs needs a complete rewrite from scratch bringing them all together into one new program.

Here is a brief explanation of the current differences

Here is the main thread BlackAce started for OGX

Here’s my thread starting the discussion on a rewrite

2 Likes