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

axis<n>.motor.error request on ASCII protocol always returns "lu" #571

Open
pbrier opened this issue May 21, 2021 · 10 comments
Open

axis<n>.motor.error request on ASCII protocol always returns "lu" #571

pbrier opened this issue May 21, 2021 · 10 comments
Labels

Comments

@pbrier
Copy link
Contributor

pbrier commented May 21, 2021

Describe the bug
In the latest "release candiate" firmware (e.g. v0.5.2 release #562) , Requesting the axis motor error does not respond with a number, but a string "lu" instead.

To Reproduce
issue the command "r axis0.motor.error\n" on any serial/USB connection to the drive with the ASCII protocol.

Expected behavior
The axis should respond with a number (e.g. "0\n")

Desktop (please complete the following information):

  • Ubuntu linux
  • arm-none-eabi-gcc (15:9-2019-q4-0ubuntu1) 9.2.1 20191025 (release) [ARM/arm-9-branch revision 277599]
  • Any comm program (e.g. putty)

Additional context
There are additional error flags defined for the motor in the recent release, that cause the datatype of the motor error to go from uint32 to uint64. Formatting uint64 values with the linked library does not seem to to work. "%llu" outputs "lu" instead of the uint64 argument. Seems related to this: ARMmbed/mbed-os#6137

Probably other uint64 properties (line drv_fault) are also incorrectly displayed when requested.

@pbrier pbrier added the bug label May 21, 2021
@pbrier
Copy link
Contributor Author

pbrier commented May 21, 2021

Seems indeed related to newlib-nano. When compiling without nano specs it indeed reports a "0" on the axis.motor.error.
When replacing this in Tupfile.ini:

-- LDFLAGS += '-mthumb -mfloat-abi=hard -specs=nosys.specs -specs=nano.specs -u _printf_float -u _scanf_float -Wl,--cref -Wl,--gc-sections'
LDFLAGS += '-mthumb -mfloat-abi=hard -specs=nosys.specs -u _printf_float -u _scanf_float -Wl,--cref -Wl,--gc-sections'`

This works as expected. Do not know if this breaks the whole firmware (creates larger/slower binary). Alternative would be to squeeze the error bits in 32bit by using currently unused bits in the enum (there are only 27 bits used at the moment)

@pbrier
Copy link
Contributor Author

pbrier commented May 27, 2021

Hmm. Not using the newlib nano.specs causes the "w" command of ints to fail with a "not implemented" response.
(e.g. "w axis.requested_state 1" --> "not implemented") so something needs to give... what would be the suggested direction for a solution (making the embedded code handle 64 bit ints properly or avoiding them)

@PAJohnson
Copy link
Collaborator

PAJohnson commented May 28, 2021

I've just tested ascii with newlib-nano removed and w commands work - for example, w axis0.requested_state 4\n works as expected and r axis0.motor.error\n returns a number instead of lu. I'm using putty for this - does this issue still happen for you on the latest commit to the fw-v0.5.2rc1 branch?

@pbrier
Copy link
Contributor Author

pbrier commented May 31, 2021

Yes. Pulled latest version and Checked with and without the newlib spec. 100% reproducible result. With newlib.spec: command accepted, without newlib spec "not implemented". Could this be related to the gcc-arm compiler version I use?
What version do you use to compile?
It seems to go haywire here This does not return "1" for the requested type. The format_trait_t seem to be off here. This also explains why I get an extra "d" if I request "axis.error" (response is "0d\n") when I compile without nano.spec. Could this be a byte enum that has an incorrect format trait definition? Might be related to how newlib is compiled for the specific ARM-GCC version. What is your drive's response to "r axis.error\n" with and without newlib nano?

@pbrier
Copy link
Contributor Author

pbrier commented May 31, 2021

An would using the full newlib be wise? It takes a 2k RAM penalty (and ~7K flash). In addition: using scanf() and printf() on a resource constrained platform could be problematic in general IMHO. Can we avoid it completely? Or at least stick to newlib nano? (preventing int64 or providing our own formatting/scanning)? What do you think?

compiled with nano.specs:
text data bss dec hex filename
288512 1856 142312 432680 69a28 build/ODriveFirmware.elf

Compiled witout nano.specs:
text data bss dec hex filename
305192 3872 141944 451008 6e1c0 build/ODriveFirmware.elf

@pbrier
Copy link
Contributor Author

pbrier commented May 31, 2021

I confirmed the problem is indeed also related to my local arg-gcc compiler! Using the dockerbuild.sh script solves the problem of setting the axis state (without nano.specs both reading and writing works as expected).

Could be that the docker has a newlib that is compiled with --enable-newlib-io-long-long and and --enable-newlib-io-c99-formats. Where mine does not have that. Seems the ARM version of gcc-arm does not have it on by default.

docker build container uses GNU C11 (GNU Tools for Arm Embedded Processors 7-2018-q3-update) version 7.3.1 20180622 (release) [ARM/embedded-7-branch revision 261907] (arm-none-eabi) compiled by GNU C version 7.3.0, GMP version 6.1.2, MPFR version 4.0.1, MPC version 1.1.0, isl version isl-0.18-GMP

But "r axis.error\n" still responds with "0d\n"!

Is it wise to fix this (without nano.specs) or fix the 64bit int problem with nano.specs?

@pbrier
Copy link
Contributor Author

pbrier commented May 31, 2021

Having tested various versions of the ARM compiler and with nano.spec and without I wonder why not remove the root cause of this problem? The int64 for the error flags. If the fixed bit assignments of the errors are removed, they fit neatly in 32bits, and the CAN protocol can also remain unchanged. This may break backwards compatibility with users of the ascii protocol if they do not update their enums, but it is risky anyway to assume values.

@Wetmelon
Copy link
Collaborator

Wetmelon commented Sep 13, 2021

Hi @pbrier is this still an issue with new releases (10+) of arm-none-eabi-gcc?

@pbrier
Copy link
Contributor Author

pbrier commented Sep 13, 2021

Have to check. You mean arm-gcc 10-2020-q4-major for example?
nano.spec will always be an issue I think for 64bit ints (limitation of library).

@dushabella
Copy link

@Wetmelon problem still appears in 10.3-2021.07 version of arm-none-eabi-gcc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants