openinverter forum

Thanks Matt will do.

That's wired, I need to think how it's going into the next byte. Also the lsb byte 26 is zero in each case, which seems odd.

Them two lines look ok to me, I would often do similar. Also this seemed to work ok on the old processor, which was also a 32bit arm, code compiled on gcc.

The brackets don't seem to be doing much in the original code. I'd normally write that conversion as follows:

htm_data[26]=(uint8_t) (mg2_torque&0xFF);
htm_data[27]=(uint8_t) (mg2_torque>>8);

Yes that'll do. Data doesn't overflow to the next byte, whatever you do. The LSB is used in these byte assignments.
You could be extra naughty and write
*(int16_t*)(&htm_data[26])=mg2_torque;

Or wrap the entire array into a __packed__ struct

I think cppcheck would definitely be mad at either of those.

Then I wonder where he is seeing data on [28] from? (I don't have the code in front of me)

I wonder is the dma out by a byte? Hence what's at index 28 should be index 27?

170 hex = 368,

Could there be a x 10 scaling issue with the torque?

Assuming we're talking about this: https://github.com/damienmaguire/Stm32- ... GS450H.cpp

What I mean by packed struct is something like this: Now all the fields are cleanly accessible via the struct members and does away with all the bit-banging. packed attribute makes sure that the compiler doesn't add any padding by itself.

AAAAAAAAAAAAAHHHHHHHHHHHHHHHHHHHHHHHHHHHH NNNNOOOOOOOOOOOOOOOOOOOOOOOOOOO

Ok, now that we got that out of the way something interesting is going on. Mg1 spins perfectly, to high rpms, in both directions. MG2 still unhappy at much over a torque value of.........wait for it........127!

Yea so when it goes from 7 bits to 8 bits. Is it on the LSB or MSB going above 127 where the problem happens?

It is strange as the same conversion is being done on MG1 and MG2, although one has the sign flipped.

I assume the Param::Torque on the web-interface is printing correctly?

Got it!
was missing from the htm packet and causing the inverter to torque limit mg2 but not mg1. Why? Well we need mg1 to start the engine to charge the frigging battery:) AAAAHHHHHHHHHHHHHHHHHHHHHH

Oh brilliant, good spot, that's amazing.

The E65 is rolling with the ZombieVerter VCU:) turns out the twitching tacho was a Damien special : signed value in an unsigned variable...

well done, that's excellent

annnnnddd we can shift gears:) this thing is a beast in Low! Going to map the steering wheel shift buttons to the shift routine as soon as I put some overspeed safeguards in there. I wonder is there an lsd that would fit the E65.......

anyway, thanks everyone who has / is helping. Next up : MG temp sensors then onto the E46/Leaf combo.

Excellent. Awesome news on the gear shifting working! Now we're at a good point to breathe a little.

https://releases.llvm.org/10.0.0/tools/ ... tions.html - I think standard LLVM coding guide would keep most people happy?

Then we can see what other fun data-type problems lie under the surface.

-Matt

Jack Bauer wrote: ↑Fri Jan 08, 2021 3:48 pm annnnnddd we can shift gears:) this thing is a beast in Low! Going to map the steering wheel shift buttons to the shift routine as soon as I put some overspeed safeguards in there. I wonder is there an lsd that would fit the E65.......

anyway, thanks everyone who has / is helping. Next up : MG temp sensors then onto the E46/Leaf combo.

I had implemented the mg temp sensor reading on the old ecu I'll feed over what I did, should port over easily

PR created for initial cleaning: https://github.com/damienmaguire/Stm32-vcu/pull/1 (This was done in CodeBlocks, not via clang-format. Need to figure out how to make them both coexist together )

* Binary files excluded correctly
* Initial formatting to make it readable outside of CodeBlocks.

I have some further ideas, but need to do the day job at the moment.

-Matt

Thanks Matt. Merged in.

This is the function to read the thermistors in the gearbox, I guess maybe put it in a 1 second task?

float vcc = 5.0;
float adc_step = 3.3/1023.0;
float Rtop = 47000.0;
float Ro = 47000;
float To = 25+273;
float B = 3500;

float readThermistor(int adc){

float raw = adc;
float voltage = raw*adc_step;

float Rt = (voltage * Rtop)/(vcc-voltage);

float temp = (1/(1.0/To + (1.0/B)*log(Rt/Ro)))-273;

return temp;
}


I had changed the resistor value for the pullup in the ECU i was using to the 47K as that gave the best resolution around 25-50C. But we could just change the Rtop value above to 1800 to match the new hardware. Also i believe we have 12bit ADCs enabled, so i've made that change too:-


float vcc = 5.0;
float adc_step = 3.3/4096.0;
float Rtop = 1800.0;
float Ro = 47000.0;
float To = 25+273;
float B = 3500;

The closest curve i could find for the thermistor is shown here:-
viewtopic.php?f=14&t=396&p=7319&hilit=M ... stor#p7319

PR opened, but needs testing.

Please test:
https://github.com/mdrobnak/Stm32-vcu/tree/e46_update_1

From my look at it you were comparing pre-scaled RPM values against an input which wasn't scaled.

I also added more color to your data.


Edit:
Separate PR, this should be good to go:
https://github.com/damienmaguire/Stm32-vcu/pull/3


-Matt

Well, I broke git:(

Tried to push some updates today and it went crazy talking about branches and heads and pulls and filled my cpp files with red arrows and weird numbers.

That means there is a difference between the state got expected and what you provided.

The awesome thing about git is that as long as you didn't force push (only specific scenarios to, which....not yet hehe) anyone who has a copy of the repo can likely fix it.

I can try if you give me write access.

For now, try pushing to a test branch:

git push origin master:test

It wants you to resolve conflicts it can't resolve on it's own. That's what the arrows are all about. Local vs remote versions.

-Matt

Think I have sent you an invite on github Matt. In other news the Grey Goose arrived home today so E46 and Nissan Leaf testing on the vcu is next:) The Goose also has the M3 pcs fitted so will see about finally waking that up and integrating into the vcu.