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

Legacy and Outback port #717

Open
wants to merge 30 commits into
base: devel
from

Conversation

@bugsy924
Copy link

bugsy924 commented Jun 30, 2019

No description provided.

@geohot geohot added the car port label Jul 1, 2019
@geohot

This comment has been minimized.

Copy link
Contributor

geohot commented Jul 1, 2019

Can you do the panda PR on the panda repo?

@bugsy924

This comment has been minimized.

Copy link
Author

bugsy924 commented Jul 1, 2019

It's riccardo's already open PR
commaai/panda#189

@rbiasini

This comment has been minimized.

Copy link
Contributor

rbiasini commented Jul 1, 2019

Yes, I'll fix the conflicts and merge:
commaai/panda#189

self.v_cruise_pcm *= CV.MPH_TO_KPH
else:
self.v_cruise_pcm = cp_cam.vl["ES_DashStatus"]["Cruise_Set_Speed"]
self.steer_not_allowed = cp.vl["Steering_Torque"]["LKA_Lockout"]

This comment has been minimized.

Copy link
@rbiasini

rbiasini Jul 1, 2019

Contributor

this breaks the Impreza. All the cars shall have the same attributes.

This comment has been minimized.

Copy link
@bugsy924

bugsy924 Jul 1, 2019

Author

Which lines break the Impreza, 163-169? Should be the same. I'll look into it more later. The Outback and Legacy are on a very different platform

This comment has been minimized.

Copy link
@bugsy924

bugsy924 Jul 1, 2019

Author

I'll change that else to outback and legacy

This comment has been minimized.

Copy link
@rbiasini

rbiasini Jul 1, 2019

Contributor

steer_not_allowed is used in interface. so needs to be defined for all cars

This comment has been minimized.

Copy link
@bugsy924

bugsy924 Jul 1, 2019

Author

Ah I see now, I'll fix these all tonight. Thanks

This comment has been minimized.

Copy link
@bugsy924

bugsy924 Jul 3, 2019

Author

I made the changes
Would you like me to redo this on 0.6?

{
2: 8, 64: 8, 65: 8, 72: 8, 73: 8, 256: 8, 280: 8, 281: 8, 290: 8, 312: 8, 313: 8, 314: 8, 315: 8, 316: 8, 326: 8, 372: 8, 544: 8, 545: 8, 546: 8, 554: 8, 557: 8, 576: 8, 577: 8, 722: 8, 801: 8, 802: 8, 805: 8, 808: 8, 811: 8, 826: 8, 837: 8, 838: 8, 839: 8, 842: 8, 912: 8, 915: 8, 940: 8, 1614: 8, 1617: 8, 1632: 8, 1650: 8, 1657: 8, 1658: 8, 1677: 8, 1697: 8, 1759: 8, 1786: 5, 1787: 5, 1788: 8
}],
# OUTBACK PREMIUM 2.5i 2015
CAR.OUTBACK: [{
2: 8, 208: 8, 209: 4, 210: 8, 211: 7, 212: 8, 320: 8, 321: 8, 324: 8, 328: 8, 329: 8, 336: 2, 338: 8, 342: 8, 346: 8, 352: 8, 353: 8, 354: 8, 356: 8, 358: 8, 359: 8, 392: 8, 640: 8, 642: 8, 644: 8, 864: 8, 865: 8, 866: 8, 872: 8, 880: 8, 881: 8, 882: 8, 884: 8, 977: 8, 1632: 8, 1745: 8, 1786: 5

This comment has been minimized.

Copy link
@rbiasini

rbiasini Jul 1, 2019

Contributor

indent style is different here

@@ -143,6 +150,7 @@ def update(self, cp, cp_cam):
self.right_blinker_on = cp.vl["Dashlights"]['RIGHT_BLINKER'] == 1
self.seatbelt_unlatched = cp.vl["Dashlights"]['SEATBELT_FL'] == 1
self.steer_torque_driver = cp.vl["Steering_Torque"]['Steer_Torque_Sensor']
self.steer_torque_motor = cp.vl["Steering_Torque"]['Steer_Torque_Output']

This comment has been minimized.

Copy link
@rbiasini

rbiasini Jul 1, 2019

Contributor

unused?

This comment has been minimized.

Copy link
@bugsy924

bugsy924 Jul 1, 2019

Author

Unused now, yes, I can remove it
Previously used when using the same steer limit as Toyota

bugsy924 added 2 commits Jul 2, 2019
Updated Outback tuning
Fixed fingerprint indentation
Removed steer_torque_motor
Added steer_not_allowed = 0 to Impreza
@bugsy924 bugsy924 closed this Jul 13, 2019
@bugsy924 bugsy924 deleted the bugsy924:Subaru-PR branch Jul 13, 2019
@bugsy924 bugsy924 restored the bugsy924:Subaru-PR branch Jul 13, 2019
@bugsy924

This comment has been minimized.

Copy link
Author

bugsy924 commented Jul 13, 2019

That close/delete was accidental

@bugsy924 bugsy924 reopened this Jul 13, 2019
bugsy924 added 3 commits Sep 10, 2019
self.STEER_DRIVER_ALLOWANCE = 300 # allowed driver torque before start limiting
self.STEER_DRIVER_MULTIPLIER = 1 # weight driver torque heavily
self.STEER_DRIVER_FACTOR = 1 # from dbc
self.STEER_DELTA_DOWN = 60 # torque decrease per refresh

This comment has been minimized.

Copy link
@rbiasini

rbiasini Sep 12, 2019

Contributor

mmm... not using the impreza params will be quite difficult to manage in panda safety I think. At the moment, how does it work in your fork?

This comment has been minimized.

Copy link
@bugsy924

bugsy924 Nov 1, 2019

Author

The message length is 1 bit longer on pre-global. I don't currently know how to make it work, I think I cheated with the bit shifting for the legacy and outback to trim the lsb to match impreza length to not enable a violation. It would have been great if subaru kept the same CAN bus IDs but it's almost a separate make port

@bugsy924

This comment has been minimized.

Copy link
Author

bugsy924 commented Sep 12, 2019

75 up and down are the limits the EPS seem to follow. I'll change this to follow impreza.

self.STEER_DRIVER_FACTOR = 1 # from dbc

self.STEER_DRIVER_FACTOR = 1 # from dbc
self.STEER_DRIVER_ALLOWANCE = 60 # allowed driver torque before start limiting

This comment has been minimized.

Copy link
@rbiasini

rbiasini Oct 30, 2019

Contributor

we probably need to distinguish between Impreza and others and pass this param to panda safety.

idx = (frame / steer_step) % 8
steer2 = (chksm_steer >> 8) & 0x1F
steer1 = chksm_steer - (steer2 << 8)
checksum = (idx + steer2 + steer1 + chksm_engage) % 256

This comment has been minimized.

Copy link
@rbiasini

rbiasini Oct 30, 2019

Contributor

I would define a generic "subaru_checksum_legacy" function. It must be computed in some generic way...

This comment has been minimized.

Copy link
@bugsy924

bugsy924 Nov 1, 2019

Author

I'll try, it's just the sum of all bytes mod 256.
I need to do it anyway because the majority of the messages use the same checksum including the button control message and I need that for cancelling without using doors and for stop+go auto-resume

bugsy924 added 7 commits Nov 9, 2019
* Standardised checksum calculation for preglobal
* Changed open door disengage to mocking cancel button (intercepted message, not flooding)
* Added Outback 3.6r 2018 fingerprint
* Changed STEER_THRESHOLD
* Updated DBC
* Filtered 353 ***CHECK IF THIS CONFLICTS WITH GLOBAL***
Fixed accidental swap from legacy fingerprint to crosstrek
Removed some leftovers
Conflicted with the outback 3.6r fingerprint
added transmission type
added ret.openpilotLongitudinalControl = False
I can't imagine this is why travis ci is failing
bugsy924 added 16 commits Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.