Skip to content
This repository was archived by the owner on Feb 3, 2023. It is now read-only.

Allow varying units in printing odomState#487

Merged
Octogonapus merged 4 commits into
OkapiLib:masterfrom
karmanyaahm:upstream-master
Jun 26, 2022
Merged

Allow varying units in printing odomState#487
Octogonapus merged 4 commits into
OkapiLib:masterfrom
karmanyaahm:upstream-master

Conversation

@karmanyaahm
Copy link
Copy Markdown
Contributor

@karmanyaahm karmanyaahm commented Jun 10, 2022 •
edited
Loading

Test this with:

void opcontrol()
{
        using namespace okapi::literals;
        auto o = okapi::OdomState{x : 4.20_tile, y : 31.4_in, theta : 69_deg};

        try
        {
                printf("%s\n", o.str().c_str()); //normal, compatible with existing behavior
                printf("%s\n", o.str(1_ft, 1_deg).c_str()); // basic unit specification
                printf("%s\n", o.str(okapi::meter, okapi::radian).c_str()); // another basic unit specification
                printf("%s\n", o.str(6_tile / 100, "%", 360_deg / 100, "%").c_str()); // fancy custom unit specification (% of field)
                printf("%s\n", o.str(1_m, 1.01_deg)); // should fail because no matching unit
        }
        catch (const std::domain_error &e)
        {
                printf("%s\n", e.what());
        }
}

TODO:

  • docs
  • Rebase: clean up commit history

Closes #467

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 10, 2022 •
edited
Loading

Codecov Report

Merging #487 (358eb9d) into master (9e8c894) will increase coverage by 0.02%.
The diff coverage is 66.67%.

@@            Coverage Diff             @@
##           master     #487      +/-   ##
==========================================
+ Coverage   89.11%   89.13%   +0.02%     
==========================================
  Files         139      140       +1     
  Lines        6143     6155      +12     
==========================================
+ Hits         5474     5486      +12     
  Misses        669      669

karmanyaahm
karmanyaahm commented Jun 10, 2022
View reviewed changes
Comment thread include/okapi/api/units/RQuantityName.hpp
@karmanyaahm
Copy link
Copy Markdown
Contributor Author

karmanyaahm commented Jun 10, 2022

@Octogonapus Can you please answer the questions in the review comments?

karmanyaahm
karmanyaahm commented Jun 10, 2022
View reviewed changes
Comment thread src/api/odometry/odomState.cpp
@karmanyaahm
Units: Add getShortUnitName to get string suffix
100cfec 
The function std::string getShortUnitName(QType q) in
include/okapi/api/units/RQuantityName.hpp returns the a string of the
matching unit.

For example, a call such as `getShortUnitName(1_m)` will return "m",
1_rad will return "rad". A value that doesn't exactly (within 0.01%) map
to a unit such as 1.01_m will throw an error.

This is currently only implemented for QLength and QAngle because that's
what's needed for Odometry. It should be trivial to add other types to
the map if the need arises in the future.
@karmanyaahm
Odometry: OdomState::str can now customize units
c19a50a 
There are two versions of the function now,
- one that takes in units, and uses the getShortUnitName function to
  determine the suffix for thestring representation of that value.
- The other takes in a unit to convert to, but uses user input strings
  as the suffix. This allows the caller to use fancy units, or ignore
  units entirely.
- The no arguments, backwards compatible caller uses the latter with
  default arguments of meter, "_m", and degree, "_deg". This default
  function removes the need for map lookups on the default function,
  which the former would require.

Example calls to this function would be:
- `OdomState::str()`: returns values in meters and degrees, with _m and
  _deg suffixes
- `OdomState::str(1_ft, 1_rad)`: returns values in feet and radians, with
  getShortUnitName determining the appropriate suffixes (_ft and _rad)
- `OdomState::str(6_tile / 100, "%", 360_deg / 100, "%")`: is a fancy
  way of getting the values in % of the vex field, and % degrees of a
  full circle.
- `OdomState::str(1_m, 1.01_deg)`: should fail because it uses unit
  lookup on an undefined unit (1.01_deg has no unit suffix).
@karmanyaahm karmanyaahm marked this pull request as ready for review June 15, 2022 01:27
@karmanyaahm karmanyaahm requested a review from Octogonapus June 15, 2022 01:28
@karmanyaahm
Copy link
Copy Markdown
Contributor Author

karmanyaahm commented Jun 25, 2022

@Octogonapus can you look at this please?

Octogonapus
Octogonapus approved these changes Jun 26, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@Octogonapus Octogonapus left a comment

Choose a reason for hiding this comment

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

pretty sure you need double backticks for inline code but we'll see

@Octogonapus Octogonapus merged commit d775346 into OkapiLib:master Jun 26, 2022
@Octogonapus
Copy link
Copy Markdown
Member

Octogonapus commented Jun 26, 2022

update: no that actually works

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@Octogonapus Octogonapus Octogonapus approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow users to set units for OdomState string representation

2 participants

@karmanyaahm @Octogonapus