bugfix/header-errors#11
Conversation
| #define MAX_BUF 16 | ||
| #define MAX_LOG_LEN 128 | ||
| extern FsFile logFile; | ||
| #define SERIAL_DEBUG true |
There was a problem hiding this comment.
in earlier iterations of code i wrote for the teensy, i defined serial debug not as a boolean true or false, but as a number 0 through 4 or 5, so i could have varying levels of verbosity
That means you could still have statements like this
if(SERIAL_DEBUG) {
Serial.print("Value:");
Serial.println(some_value)
}But also like level-specific guards:
if(SERIAL_DEBUG >= 2) {
Serial.print("Detailed value: ");
Serial.println(some_value);
}This way SERIAL_DEBUG 0 is silent, 1 gives high-level status messages, and higher levels progressively expose more granular internals. The boolean check still works at level 1+ since any non-zero value is truthy in cpp
During drive SERIAL_DEBUG should be set to 0 before compilation. Compiler should treat the if blocks as dead code and omit them from the compiled binary, but worth looking into the default optimisation values for teensy
platformio.ini supports build_flags so may be worth explicitly setting build_flags = -02 for speed optimisation
|
@swhelan123 not letting me merge, adding new changes to this pr |
swhelan123
left a comment
swhelan123
left a comment
There was a problem hiding this comment.
small changes requested. see comments. maybe i'm being stupid about the LogLevel gripes, pls explain if i am
| static LogLevel serialDebug; | ||
|
|
||
| public: | ||
| static CircularBuffer<LogEntry, MAX_BUF> _logBuffer; | ||
| static bool begin(); | ||
| static bool begin(LogLevel debugLevel); |
There was a problem hiding this comment.
having "debug" in the variable name for LogLevel types could get confusing since DEBUG is a level itself. Also serialDebug implies pertinence to the serial channel even though LogLevel only concerns SD card datalogging
SERIAL_DEBUG_LEVEL or something along those lines could be used as a compile time macro and accessed globally to increase or decrease verbosity of output to serial monitor but that is out of the scope of the logger class
| bool Logger::begin(LogLevel serialDebugLevel) { | ||
| serialDebug = serialDebugLevel; |
There was a problem hiding this comment.
same comment as above. serial has nothing to do with SD card data logging in this context
|
|
||
| LogLevel serialDebug = LogLevel::NONE; | ||
| Logger::begin(serialDebug); | ||
| if (serialDebug != LogLevel::NONE) { |
There was a problem hiding this comment.
ok now i'm confused if logger has to do with sd card data logging or live serial output or both. if the latter, consider splitting up responsibility of the logger class to an SD logger and a live debug logger?
| // Plausibility: sensors must agree within PEDAL_PLAUSIBILITY_PERCENT. | ||
| // Fault only latches after 3 consecutive bad readings (~60ms) to reject | ||
| // single noisy samples. Zero torque immediately on any bad reading. | ||
| if (fabsf(pct1 - pct2) > PEDAL_PLAUSIBILITY_PERCENT) { | ||
| plausibilityCount++; | ||
| if (plausibilityCount >= 3) fault = true; | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
✅ T11.8.9 - implausibility is defined as >10 percentage points deviation between any two APPS sensors.
❌ T11.8.8 - if an implausibility persists for more than 100ms, power to the motor(s) must be immediately shut down completely. Shutting down via the motor controller is sufficient - you don't have to open the SDC, but the inverter must cut power.
11.8.9 is implemented fine with the macro, 11.8.8 should be implemented to comply completely with rules
There was a problem hiding this comment.
is this where brake pedal pressure sensor inputs will be handled in the future too? if yes, add placeholders to ensure we do not forget to implement the following
EV2.3.1 — if brakes are mechanically actuated AND APPS signals >25% pedal travel (or >5kW, whichever is lower) simultaneously for more than 500ms, commanded torque must be 0 Nm.
EV2.3.2 — once that condition triggers, torque must stay at 0 Nm until APPS drops below 5% and 0 Nm is commanded — regardless of whether brakes are still on.
|
|
||
| float pct = (pct1 + pct2) * 0.5f; | ||
|
|
||
| if (pct < PEDAL_DEADBAND_PERCENT) { |
There was a problem hiding this comment.
could you do what you did below doing pct = std::max(0, PEDAL_DEADBAND_PERCENT?
i suppose that would mirror the same logic as what you've got here, but does that mean as you increase pedal it goes 0...0...3 all of a sudden?
There was a problem hiding this comment.
the reason for the deadband in the old code is that when you configure rest to say 2000 and full to 1000, a bit of noise may make rest go to like 1956 or 2083 or something so rather than throwing errors if raw reading goes outside set range (drops to 1956) or send a small percentage torque request (goes to 2083) a small deadband is set around rest so this noise can be dealt with. would still obviously like to preserve a full 0-100 operational window though
3 participants
Put missing #pragma once to some headers and added serial log option and also put logs dir in gitignore