Emit stack trace when RUN_TO_POSITION steps are done out of order

[Iris-TheRainbow]

The Lynx firmware level positional PID provided by RUN_TO_POSITION is an easy way to achieve position control without a team rolling their own PID or using one from a library. However, it does have an issue that is very easy to run in to, even on accident.

Because the position control is run at the firmware level, it is possible to "confuse" a Lynx module until power is cycled by setting the run mode for a DcMotor to RUN_TO_POSITION before you set a target position with .setTargetPosition(). Currently, the SDK basically does no handling of this error, only showing an error on the Driver Station where telemetry is found. There is no stack trace available on the Driver Station or in the robot log.

This makes it very difficult for even experienced programmers to locate where something has gone wrong. With my understanding of the SDK, the class TargetPositionNotSetException should be able to throw a stack trace or add one to the robot log, which would make it easier to correct RUN_TO_POSITION errors.

Ideally, there could be a mechanism to catch the error before it causes a fatal error on the Lynx module, not necessitating the power cycle, though that is a much more complex fix.

[gearsincorg]

My suggested alternative to throwing an exception was to automatically set the target position equal to the current position (if none had been provided). This would ensured that the RTP mode is stable at the current location, causing no unexpected motion.

Unfortunately this wasn't deemed an acceptable solution.

[Iris-TheRainbow]

I have considered this as well, but i 100% understand why it is not a great solution. I think crashing and throwing an exception here is entirely reasonable, since the code does really mess with the Lynx module when its actually sent, but a stack trace somewhere would be a massive help to troubleshooting it.

[AlecHub]

Kids have a massive learning curve as it is. I think that kids would appreciate Phil's AI solution to the problem.

[cmacfarl]

There is no one right answer.

What to do in this scenario was discussed and debated extensively among the FTC Technology Team back in the 2019 time frame. The rationale for adopting a fail fast POLA (principle of least astonishment) approach was that the described sequence of operations is a bug. As such, teaching the SDK to paper over the bug would lead to software appearing to work, but not actually doing what the user intended in the majority of cases. These sorts of bugs can lie latent for months and/or be incredibly difficult to track down and fix.

The decision was made to throw the exception in this case. This decision is not likely to be changed.

I think the thought on the DS messaging was that it would be pretty easy to search through an OpMode for any instances of setMode() to RUN_TO_POSITION so a stack trace wasn't necessarily needed, or was overlooked. However, the exception is caught, and the forced restart of the OpMode is by design. We want teams to catch this problem in their workshop, not at a competition venue. I do however think that it is not unreasonable to drop a stack trace into the log. I have taken the liberty of changing your Issue title to reflect this.

[Iris-TheRainbow]

Yea that may be a better name.

My main gripe with how the error is handled is that it is not just an OpMode restart that is forced, but that the Lynx Module is stuck in an unrecoverable state and requires a full power cycle, which can make locating issue very frustrating, especially if there are multiple instances of that issue. I do still understand that it could be considered non-ideal to try and catch that issue (the SDK would need a method to cache the target permanently and any check that before issuing the mode change command, which sounds like a messy solution), though.

Myself and anyone else who helps new programmers would love to see even just a stack trace.

[AlecHub]

Perhaps setMode() can be deprecated altogether. The RunMode is implied by:

• setPower() - doesn't use encoder
• setPowerEx() or setPowerEnc() - uses encoder
• setVelocity() - uses encoder
• setTargetPosition(int position, double power) - uses encoder
• resetEncoder(boolean stopTheMotor):
  • resetEncoder(true) - stops the motor (if you're lucky) and resets the encoder
  • resetEncoder(false) - resets the encoder without stopping the motor

[Iris-TheRainbow]

unfortunately, that doesn't work because the run modes are firmware level, not in the SDK itself.

[AlecHub]

What I'm saying is the SDK can set or switch the run mode of the firmware automatically if that is what is required by the particular firmware or motor controller.

[Iris-TheRainbow]

That would be a layer of hiding things the sdk shouldn’t be doing.

…

On Tue, Dec 3, 2024 at 5:09 PM Alec ***@***.***> wrote: What I'm saying is the SDK can set or switch the run mode of the firmware automatically if that is what is required by the particular firmware or motor controller. — Reply to this email directly, view it on GitHub <#1345 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AUYOI6GEW5LFAZYOTU5CQOL2DY24FAVCNFSM6AAAAABS37U5QKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMJVG42TQMZWGI> . You are receiving this because you authored the thread.Message ID: ***@***.*** com>

[AlecHub]

That would be a layer of hiding things the sdk shouldn’t be doing.

Sorry, does not compute.

[cmacfarl]

Perhaps setMode() can be deprecated altogether. The RunMode is implied by:

Also discussed extensively by the tech team over the years.

[AlecHub]

Perhaps setMode() can be deprecated altogether. The RunMode is implied by:

Also discussed extensively by the tech team over the years.

Good to know @cmacfarl. I do see and appreciate the merits of explicitly having to change the RunMode. Teams always have the option to create their own interface classes and methods to the SDK, to simplify and manage any difficulties they are having with managing the RunModes.

[Iris-TheRainbow]

Closing as release 10.2 seems to fix.