Open-Agriculture / AgIsoStack-plus-plus

AgIsoStack++ is the completely free open-source C++ ISOBUS library for everyone
https://agisostack.com/
MIT License
168 stars 40 forks source link

[TC]: Add a helper for getting implement geometry from a DDOP #427

Closed ad3154 closed 4 months ago

ad3154 commented 5 months ago

Describe your changes

Added a helper to easily get implement geometry from a DDOP, which would be nice to have in a TC server.

This currently provides a static function get_implement_geometry which returns a vector of Booms. Each boom then contains descriptions of that boom's geometry like this:

Each value itself contains metadata so the user knows if the value is settable or not, and a way to check if the value is known or not (based on it being in a property versus being in a DPD for example).

You can check if values were provided by doing things like if (booms.at(0).xOffset_mm) for example, then check the value itself booms.at(0).xOffset_mm.get()

Take a look at the little unit test I added to see more.

Questions

I would like some feedback on what specific additional info would be desirable from this helper!

For example, if we want to provide product information, what's important to know about products? There are so many different kinds of rates and things that it is difficult to abstract to some extent.

Do we want these values to be settable by the users so that they can track value updates on DPDs with them?

How has this been tested?

Added a unit test for this parsing based on our seeder example. I intend to also add one additional DDOP to test against before merging so that the sub-boom geometry gets tested.

Added three unit tests which validate all 3 "boom" configurations.

sonarcloud[bot] commented 5 months ago

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

6 New issues
0 Security Hotspots
91.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

GwnDaan commented 4 months ago

For example, if we want to provide product information, what's important to know about products? There are so many different kinds of rates and things that it is difficult to abstract to some extent.

I'm unsure what specific product information you are targeting here. But I think making a helper class for abstracting the rate control can be a good start here. Implementing the setpoint, actual, default, minimum and maximum application rate for all the different types with yet common values (uint32). Then based on like an enum or something we can differentiate them. Ideally we have it automatically track the state by listening to changes to the respective DDEntities, similar to the VT tracker helper. And then also for the other way around, we can have some setter functions that abstracts the setting of these application rates, similar to the VT update helper... Just thinking out loud here

ad3154 commented 4 months ago

I think I'll merge for now, and follow-up to fix your nits, as well as to add some extra data for sections, like I think it would be nice to also return the element numbers of each section so a TC knows quickly what section == a specific element to command. Reduces the need for further O(n^2) searches.

As for products, it's just very complicated, because you can have a boom level rate, for example, with sub-rates in each sub-boom, with sub-rates in each section, and with booms having different kinds of rates, like count/area, volume/area, mass/area etc, so abstracting it seems like it will be tough to represent well.. I will experiment around...