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

Mazda: Slower rate for ACC cancel msg to avoid cruise disable #1615

Merged
merged 2 commits into from
Jun 1, 2020

Conversation

Jafaral
Copy link
Contributor

@Jafaral Jafaral commented Jun 1, 2020

Sending ACC cancel message at 10hz may end up disabling
main cruise state. That is because stoc ACC use the same
signal for both functions. If cruise is already enabled
the signal canceles it, otherwise cruise is disabled.
10hz seems to be still fast in some cases not allowing
the state to sync up between OP and the Stock state.
5hz avoids that issue. Also, the resume message was also
updated to 5hz as that seems to work better as well.

Signed-off-by: Jafar Al-Gharaibeh to.jafar@gmail.com

 Sending ACC cancel message at 10hz may end up disabling
main cruise state. That is because stoc ACC use the same
signal for both functions. If cruise is already enabled
the signal canceles it, otherwise cruise is disabled.
10hz seems to be still fast in some cases not allowing
the state to sync up between OP and the Stock state.
5hz avoids that issue. Also, the resume message was also
updated to 5hz as that seems to work better as well.

Signed-off-by: Jafar Al-Gharaibeh <to.jafar@gmail.com>
@@ -23,17 +23,17 @@ def update(self, enabled, CS, frame, actuators):
CS.out.steeringTorque, SteerLimitParams)
self.steer_rate_limited = new_steer != apply_steer

if CS.out.standstill and frame % 50 == 0:
if CS.out.standstill and frame % 20 == 0:
Copy link
Contributor

@adeebshihadeh adeebshihadeh Jun 1, 2020

Choose a reason for hiding this comment

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

Can you use DT_CTRL instead? It's in common.realtime

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not familiar with it pointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to leave this as is for now @adeebshihadeh as it works fine now. DT_CTRL will require more code changes and more test drives which delay things if we want to get these fixes in 0.7.6.

Signed-off-by: Jafar Al-Gharaibeh <to.jafar@gmail.com>
@adeebshihadeh adeebshihadeh merged commit 41fbce5 into commaai:master Jun 1, 2020
pd0wm pushed a commit that referenced this pull request Jun 1, 2020
* Slower rate for ACC cancel msg to avoid cruise disable

 Sending ACC cancel message at 10hz may end up disabling
main cruise state. That is because stoc ACC use the same
signal for both functions. If cruise is already enabled
the signal canceles it, otherwise cruise is disabled.
10hz seems to be still fast in some cases not allowing
the state to sync up between OP and the Stock state.
5hz avoids that issue. Also, the resume message was also
updated to 5hz as that seems to work better as well.

Signed-off-by: Jafar Al-Gharaibeh <to.jafar@gmail.com>

* Round up 27.96 to 28mph

Signed-off-by: Jafar Al-Gharaibeh <to.jafar@gmail.com>
@Jafaral Jafaral deleted the acc-cancel branch November 17, 2020 05:58
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.

None yet

2 participants