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

[TC] Add helper formatters for data dictionary #535

Merged
merged 1 commit into from
Feb 6, 2025

Conversation

GwnDaan
Copy link
Member

@GwnDaan GwnDaan commented Jan 16, 2025

Describe your changes

This PR updates the data dictionary file, splits the unit variable into symbol and description, and adds display range variable. With these changes we are able to add two nice formatting methods, see comment below.

Also fixed an issue with seeder example where the reference to SetpointWorkState was missing.

How has this been tested?

Ran TC with seeder example and it formatted condensed work states:
image

@GwnDaan GwnDaan added enhancement New feature or request iso: task controller Related to the ISO-11783:10 standard labels Jan 16, 2025
@GwnDaan GwnDaan self-assigned this Jan 16, 2025

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 6 changed files in this pull request and generated no comments.

Files not reviewed (4)
  • examples/seeder_example/section_control_implement_sim.cpp: Language not supported
  • isobus/include/isobus/isobus/isobus_data_dictionary.hpp: Language not supported
  • isobus/include/isobus/isobus/isobus_standard_data_description_indices.hpp: Language not supported
  • isobus/src/isobus_task_controller_server.cpp: Language not supported
Comments suppressed due to low confidence (1)

scripts/code_gen_ddi_database.py:101

  • Ensure that rangeValues has exactly two elements before processing. Add a check to handle cases where the input format changes.
rangeValues = entityLine[2].split(' - ')
@GwnDaan GwnDaan force-pushed the daan/data-dictionary-formatters branch 2 times, most recently from 9b256c6 to 72d0991 Compare January 16, 2025 13:22
@GwnDaan GwnDaan force-pushed the daan/data-dictionary-formatters branch from 72d0991 to 89bf4ab Compare January 16, 2025 13:30
@martonmiklos
Copy link
Contributor

I wanted to ask before: have you considered pulling in an enum to string converter code like this one:
https://github.com/Neargye/magic_enum

With some smart string humanizer (camelcase to space separeted string converter) code the codes like this one:
https://github.com/Open-Agriculture/AgIsoStack-plus-plus/pull/535/files#diff-98c0e34061197b4306a7e5efbed379f62f3d43e31ad51c145932a438368feff3R73
could be greatly reduced.

@ad3154
Copy link
Member

ad3154 commented Jan 16, 2025

I wanted to ask before: have you considered pulling in an enum to string converter code like this one: https://github.com/Neargye/magic_enum

With some smart string humanizer (camelcase to space separeted string converter) code the codes like this one: https://github.com/Open-Agriculture/AgIsoStack-plus-plus/pull/535/files#diff-98c0e34061197b4306a7e5efbed379f62f3d43e31ad51c145932a438368feff3R73 could be greatly reduced.

Eh... we don't currently have any external dependencies, and it would be nice to keep it that way

ad3154
ad3154 previously approved these changes Feb 5, 2025
@GwnDaan GwnDaan force-pushed the daan/data-dictionary-formatters branch from 89bf4ab to 66d83b5 Compare February 6, 2025 12:52
@GwnDaan
Copy link
Member Author

GwnDaan commented Feb 6, 2025

Updated branch with force-push, no changes made

@GwnDaan GwnDaan merged commit 23b3501 into main Feb 6, 2025
11 checks passed
@GwnDaan GwnDaan deleted the daan/data-dictionary-formatters branch February 6, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request iso: task controller Related to the ISO-11783:10 standard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants