Added e46 config and wiki #7

jawillis · 2022-07-01T08:00:17Z

No description provided.

…Device

timurrrr

Overall looks good, just suggesting some minor corrections and a couple of minor questions.

Thanks a lot for your work!

can_db/e46.md

timurrrr · 2022-07-02T04:38:50Z

can_db/e46.md

+
+Channel name | CAN ID | Equation | Notes
+------------ | --- | -------- | -----
+Accelerator position (%) | 809 | `F * 100 / 255` |


Does this equation correctly calculate the sub-% values, or should you change to 100.0?
Alternatively, I'd just write this as a shorter F / 2.55.

timurrrr · 2022-07-02T04:49:05Z

can_db/e46.md

+------------ | --- | -------- | -----
+Accelerator position (%) | 809 | `F * 100 / 255` |
+Brake position (%) | 809 | `G & 1 * 100` | Brake pressures are not available on the main CAN bus, so this is just an on/off signal from the brake switch.
+Steering angle | 501 | `if(b & 0x80, 0 - ((b & 0x7f)<<8) \| a, (b<<8) \| a) * 0.04394` | Positive value = turning right. You can swap the equation in the if statement if you prefer it the other way around.


Is this the same as if(B & 0x80, -1, 1) * bitsToUIntLe(raw, 0, 15) * 0.04394 ?

Strange that they didn't use the two's complement for negative values.

timurrrr · 2022-07-02T04:51:02Z

configs/e46.h

+uint8_t getUpdateRateDivider(uint32_t can_id) {
+  // This is sent over the CAN bus 140 times per second and carries:
+  // vehicle speed
+  // Let's go for ~10 per second


Why so low? It's one of the most important data channels.
I suggest at least 20 updates per second for the vehicle speed.

I use 25 on 86's.

I was having throughput issues in testing and settled on 10Hz. I think this can be tuned further.

timurrrr · 2022-07-02T04:51:24Z

configs/e46.h

+  }
+
+  // This is sent over the CAN bus 140 times per second and carries:
+  // individual wheel speeds 


Ditto. Also, consider merging these two ifs together.

configs/e46.h

timurrrr · 2022-07-02T04:59:48Z

can_db/e46.md

+However, the communication protocol in RaceChrono is smart enough to optimize
+Bluetooth usage if multiple channels share the same CAN ID. As a general rule,
+if a new channel has the same CAN ID as an existing channel (such as "Throttle
+position" using the same CAN ID 320 as "Accelerator position"), then adding it


CAN ID 320 for "Accelerator position" is an 86 thing, please update with something more relevant for E46.

Yes, a cut-and-paste casualty. Will change.

timurrrr · 2022-07-02T05:00:42Z

can_db/e46.md

+Brake position (%) | 809 | `G & 1 * 100` | Brake pressures are not available on the main CAN bus, so this is just an on/off signal from the brake switch.
+Steering angle | 501 | `if(b & 0x80, 0 - ((b & 0x7f)<<8) \| a, (b<<8) \| a) * 0.04394` | Positive value = turning right. You can swap the equation in the if statement if you prefer it the other way around.
+Speed | 339 | `bitsToUIntLe(raw, 11, 13) / 16 * 0.277778` | You may want to check the multiplier against an external GPS device, especially if running larger/smaller diameter tires
+Engine RPM | 790 | `((D<<8) \| C) / 6.4` |


bytesToUIntLe(raw, 2, 2) / 6.4?

timurrrr · 2022-07-02T05:01:00Z

can_db/e46.md

+Air temperature | 1557 | `D` |
+Clutch position (%) | 809 | `D & 1 * 100` | on/off signal from the clutch switch
+Fuel level (l) | 1555 | `C & 0x7f` |
+Odometer | 1555 | `((B<<8) \| A) *10000` |


bytesToUIntLe(raw, 0, 2) * 10000?

timurrrr · 2022-07-02T05:06:13Z

configs/e46.h

@@ -0,0 +1,76 @@
+// Customizations for BMW e46


Please add
//#include "configs/e46.h"
to config.h between lines 22 and 23.

jawillis · 2022-07-02T13:31:03Z

Thanks for your feedback! I'll apply and test these changes next week when I'm back home.

jawillis · 2022-07-10T06:04:10Z

I've addressed your review comments, except for the ones related to update rates since I have not been able to get better rates with my existing nRF52-based setup. I will revisit those values when I have the ESP32 version fully tested.

timurrrr · 2022-07-26T06:45:52Z

can_db/e46.md

@@ -4,24 +4,30 @@ The CAN bus is not available at the OBD-II port, so it must be tapped into behin
 The easiest places to do this are either behind the intrument cluster or at the steering angle sensor, 
 which is located at the base of the steering column near the firewall.

+![Steering Angle Sensor Plug](../images/e46_sensor_plug.jpg)


Great photo, makes it clear where the harness is.

I just pushed 56a8228 with a re-compressed version of that photo.
Sorry I grew up in dial-up time, and still trying to save a few kB when possible 😄
You can now remove the file from this PR.

(please do keep the reference in the .md file though!)

timurrrr · 2022-07-26T06:47:10Z

can_db/e46.md


 ## Recommended CAN IDs:

 Channel name | CAN ID | Equation | Notes
 ------------ | --- | -------- | -----
-Accelerator position (%) | 809 | `F * 100 / 255` |
+Accelerator position (%) | 809 | `F / 2.54` |


Umm, why 2.54 instead of 2.55?
I presume the max value of F is 255?

timurrrr · 2022-07-26T06:48:11Z

can_db/e46.md

 Brake position (%) | 809 | `G & 1 * 100` | Brake pressures are not available on the main CAN bus, so this is just an on/off signal from the brake switch.
-Steering angle | 501 | `if(b & 0x80, 0 - ((b & 0x7f)<<8) \| a, (b<<8) \| a) * 0.04394` | Positive value = turning right. You can swap the equation in the if statement if you prefer it the other way around.
+Steering angle | 501 | `if(B & 0x80, -1, 1) * bitsToUIntLe(raw, 0, 15) * 0.04394` | Positive value = turning right. You can swap the equation in the if statement if you prefer it the other way around.


FYI I ended up flipping positive/negative in the BRZ/86 equations to match how RaceChrono visualizes the steering angle. You may also want to do left=positive.

timurrrr · 2022-07-26T06:48:58Z

configs/e46.h

-  if (can_id == 0x153) {
-    return 14;
+  if (can_id == 339) {
+    return 7;


Need to update the comment above to say 20 per second?

timurrrr · 2022-07-26T06:49:08Z

configs/e46.h

-  if (can_id == 0x1F0) {
-    return 14;
+  if (can_id == 496) {
+    return 7;


timurrrr · 2022-07-26T06:49:17Z

configs/e46.h

-  if (can_id == 0x1F0) {
-    return 10;
+  if (can_id == 790) {
+    return 5;


timurrrr · 2022-07-26T06:51:49Z

can_db/e46.md

+Constant/switched power and ground can also be found at this connector.  The harness pictured does not include the 
+power/ground.  
+
+![CAN bus harness](../images/e46_harness.jpg)


It's very hard to see the wire colors on that photos, presumably due to incandescent light source?
Can you try to take a photo outside in bright sunlight, or a similar light source?

Also I'd appreciate it if you can shrink/compress the file to be <300 kB.

If you don't have GIMP/Photoshop to shrink/compress the photo please let me know and I can help with that.

timurrrr · 2022-07-26T06:52:57Z

Overall looks good, just a couple of minor comments.

jawillis added 5 commits June 22, 2022 12:10

Added e46 config

802f831

Add e46 config and wiki page

82dcf8b

Update e46.md

b047ae7

revert config.h

006a016

Merge branch 'master' of https://github.com/jawillis/RaceChronoDiyBle…

f6ddb2a

…Device

timurrrr requested changes Jul 2, 2022

View reviewed changes

jawillis added 2 commits July 10, 2022 00:56

Review comments

4a8dab7

added e46.h to config.h

a50ce39

timurrrr reviewed Jul 26, 2022

View reviewed changes

configs/e46.h

if (can_id == 0x1F0) {

return 14;

if (can_id == 496) {

return 7;

Copy link

Owner

timurrrr Jul 26, 2022

Choose a reason for hiding this comment

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

ditto

timurrrr reviewed Jul 26, 2022

View reviewed changes

configs/e46.h

if (can_id == 0x1F0) {

return 10;

if (can_id == 790) {

return 5;

Copy link

Owner

timurrrr Jul 26, 2022

Choose a reason for hiding this comment

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

ditto

timurrrr reviewed Jul 26, 2022

View reviewed changes

timurrrr mentioned this pull request Aug 11, 2022

Add Support for Megasquirt Ecu #8

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added e46 config and wiki #7

Added e46 config and wiki #7

jawillis commented Jul 1, 2022

timurrrr left a comment

timurrrr Jul 2, 2022

timurrrr Jul 2, 2022

timurrrr Jul 2, 2022

jawillis Jul 2, 2022

timurrrr Jul 2, 2022

timurrrr Jul 2, 2022

jawillis Jul 2, 2022

timurrrr Jul 2, 2022

timurrrr Jul 2, 2022

timurrrr Jul 2, 2022

jawillis commented Jul 2, 2022

jawillis commented Jul 10, 2022

timurrrr Jul 26, 2022

timurrrr Jul 26, 2022

timurrrr Jul 26, 2022

timurrrr Jul 26, 2022

timurrrr Jul 26, 2022

timurrrr Jul 26, 2022

timurrrr Jul 26, 2022

timurrrr Jul 26, 2022

timurrrr Jul 26, 2022

timurrrr commented Jul 26, 2022

Added e46 config and wiki #7

Are you sure you want to change the base?

Added e46 config and wiki #7

Conversation

jawillis commented Jul 1, 2022

timurrrr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jawillis commented Jul 2, 2022

jawillis commented Jul 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timurrrr commented Jul 26, 2022