Skip to content

fixed max torque, for more actual torque - #497

Closed
Gernby wants to merge 4 commits into
commaai:develfrom
Gernby:fix-max-torque
Closed

Gernby wants to merge 4 commits into
commaai:develfrom
Gernby:fix-max-torque

Conversation

@Gernby

@Gernby Gernby commented Jan 10, 2019

Copy link
Copy Markdown

The stock torque command doesn't exceed 3840 (0xF00), and exceeding that value actually results in less torque.

@Gernby

Gernby commented Jan 10, 2019

Copy link
Copy Markdown
Author

This explains several observations I've had over the thousands of miles I've been working with OP. The most confusing observation was that the car would sometimes make a corner, and sometimes not come close. Also, when I increased the max value to 32K, I did experience some increase in response, since it allowed more "wrap around" of the bits that are used.

@rbiasini

Copy link
Copy Markdown
Contributor

How do you know this is true? did you use cabana to compare cmd torque VS electrical EPS torque?
If this is true, I'm ok merging it, but I would remove MAX_SCALE and simply adjust Kp, Ki and Kf tuning according to the new max (linear scaling would keep the control behavior roughly unchanged).

@Gernby

Gernby commented Jan 10, 2019

Copy link
Copy Markdown
Author

I haven't reviewed it in cabana yet, but I observed it with my real-time dashboard while doing a test drive on one of my standard test routes. While the response is still lacking, it's far better than I've ever experienced with this car. I was able to take corners at higher speeds with less lane wandering.
screenshot 2019-01-09 22 58 06

@Gernby

Gernby commented Jan 10, 2019

Copy link
Copy Markdown
Author

Regarding the STEER_SCALE, I agree it would be preferrable to adjust all of the other Honda's K values to eliminate the need for STEER_SCALE. I actually did something like that at first, but the response wasn't the same. Without actually calculating the values, I assumed that all of the [0.6,], [0.18,] values would "nicely" convert to the [0.8,], [0.24,] values that the CRV's and Civics use, but those values didn't work well for my Accord. Also, the standard value for all Honda's Kf is 0.00006, so it seems less desirable to split the 2 existing "tuning groups" into 4:
Existing tuning groups:
Kf = 0.00006, Kp = 0.6, and Ki = 0.18
Kf = 0.00006, Kp = 0.8, and Ki = 0.24

New tuning groups:
Kf = 0.000064, Kp = 0.64, and Ki = 0.192
Kf = 0.000064, Kp = 0.853, and Ki = 0.256

EDIT: I went the wrong way with the conversions in my original post...

@Gernby

Gernby commented Jan 10, 2019

Copy link
Copy Markdown
Author

@Gernby

Gernby commented Jan 10, 2019

Copy link
Copy Markdown
Author

I should mention the issue I raised a few weeks ago about some "episodes" I experienced relating to the stock RDM system, which prompted me to do some testing with OP in passive mode to observe the stock RDM values. The max values for RDM were 3840.
image

@Gernby

Gernby commented Jan 29, 2019

Copy link
Copy Markdown
Author

After more testing, I don't think there is a max torque value, just a max response (per time). Here are some test results while steering with a keyboard to command values multiple times higher than 0x1000. Note that the EPS responded almost identically in each direction, with 2 steering "bursts" about 5-6 seconds apart.

I'm going to close this PR, since it seems unnecessary.
screenshot 2019-01-28 16 16 45
screenshot 2019-01-28 16 48 22

@Gernby Gernby closed this Jan 29, 2019
@Gernby
Gernby deleted the fix-max-torque branch February 16, 2019 00:53
sshane referenced this pull request in sshane/openpilot Dec 4, 2019
* add ret_arne to honda.interface

* add ret_arne to chrysler.interface

* add ret_arne to rest of interfaces
sshane referenced this pull request in sshane/openpilot Dec 4, 2019
Add arne_ret to all interfaces (#497)
xx979xx pushed a commit to xx979xx/openpilot that referenced this pull request Dec 9, 2019
* updates to long_mpc

* 066 updates (commaai#208)

* add custom error messages to arne182.capnp

* add explanations for some parameters in op_params, add parameter to specify custom following distance
updates to dynamic_follow:
- fix v_lead that should have been a_lead
- made relative velocity mod more aggressive

* make customTR a bit safer

* perhaps this will work

* remove travis detected message

* start updating op_params, fix issue with lane_hugging mod where it wouldn't work with right hugging

* revert custom alert message

* testing new op_edit

* testing new op_edit

* testing new op_edit

* testing new op_edit

* testing new op_edit

* testing new op_edit

* testing new op_edit

* testing new op_edit

* testing new op_edit

* add description!

* add description!

* testing op_edit

* testing op_edit

* testing op_edit

* testing op_edit

* testing op_edit

* testing op_edit

* testing op_edit

* testing op_edit

* testing op_edit

* testing op_edit

* testing op_edit

* finish updating opEdit

* final update

* tuned dynamic follow relative velocity values to be slightly more aggressive at high rel vels
fix error in get_cost function causing it to always return 0.1 above 5 mph

* updates to long control, testing without dynamic gas

* testing customTR

* testing old cost function

* revert to 066-clean

* revert to 066-clean

* use clip instead

* updates to long_mpc

* revert TR values

* updates to long_mpc

* revert new costs to how they were in 05

* refactor, remove unneeded variable

* Create __init__.py

* At least 2m needed to keep stopping for traffic lights

* Refine one-way logic

Now both roads need to be a one way and it will only choose it if there is one way in the right direction and the other is not.

* fix for right hand turns not slowing down.

* remove integrated value on release and not press of brake/gas

* new lane hugging mod, seems to work a lot better! tuning for corolla steering, less seems to be better

* time wasn't imported

* tiny bit of refactor

* Try the reset for pid after gas pedal is

* treat brake pressed or gas pressed as LongCtrlState.off

* I also want events

* Fix hex value

* add possibility for arne182 events

* fix

* fix arne capnp

* return ret_arne182

* Try and combine events together.

This could probably not work

* think this will be better, we want to modify as little of comma's code as possible

* test

* test if messages from arne.capnp will be accepted by AM

* test combined events

* this should work

* final changes to make custom alerts work only for toyotas for now

* Stop data_send from crashing because of unknown events

* Pass in sendevents to state_transition

* try prepend

* Append lists together

* Thank goodness for syntax highlighting

* fix right hand turns

The negative value is already in steering angle which causes tan to be negative so we do not need an extra minus there.

* Completely seperate events_arne182 and events

* Add arne_ret to all interfaces (commaai#497)

* add ret_arne to honda.interface

* add ret_arne to chrysler.interface

* add ret_arne to rest of interfaces

* first version (commaai#499)

* Add exception for gas and brake press for planner

This could be the cause of the steering braking after manual gas and brake input

* Try remove red screen and loud beep at low speed reverseGear Engagement

* Add custom alert without chimeWarningRepeat

* Use new custom reverse Warning without chime

* fix create_event_arne

* Update arne182.capnp

* Fix
ccdunder pushed a commit to ccdunder/openpilot that referenced this pull request Jan 6, 2025
* Add script for squash and merge workflow automation

Introduced `squash_and_merge.py` to automate the process of squashing a source branch and merging it into a target branch. The script handles backups, commit message creation, stashing, cleanup, and includes optional push functionality. Provides user prompts and error handling to simplify complex merge workflows.

* temp1

* Switch to PEP 604 style for optional and generic type hints

Replaces `Optional` and `List` with the simpler `X | None` and `list[X]` syntax introduced in Python 3.10. Updated all function signatures accordingly for consistency and modernity. Also made minor string formatting consistency adjustments.

* Fix type annotation for temp_branch variable

Updated the temp_branch variable to include a proper type annotation (`str | None`). This change improves code clarity and aligns with Python typing standards.

* more hints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants