<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Greg, Mark,<br>
<br>
I can check your new code after work.<br>
<br>
For the TX performance/overflow issue, there are basically two
options:<br>
<ul>
<li>A: make all application TX be aware of overflows, i.e. check
the return value of the CAN Write() call as necessary and/or
introduce sufficient delays (very ugly)<br>
</li>
<li>B: add a TX queue to the CAN framework, so the application
can just push some frames as fast as it likes, with an option
to wait/block/fail if the queue is full</li>
<ul>
<li>→ the framework checks for TX buffers becoming available
(i.e. driver issuing a TxCallback request) and delivers
queued frames only as fast as the driver can handle them<br>
</li>
</ul>
</ul>
Option B has been on my todo list since removing the delay from
the MCP driver and introducing the TX buffer check in the esp32can
driver, as I don't think applications should need to handle TX
overflows.<br>
<br>
I can try to implement that this weekend if it's urgent now.<br>
<br>
Regards,<br>
Michael<br>
<br>
<br>
Am 11.01.2018 um 05:55 schrieb Greg D.:<br>
</div>
<blockquote type="cite"
cite="mid:b229b09f-ef72-c564-7839-fa8a815157cd@gmail.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
Hi Mark, Micheal,<br>
<br>
Ok, good news and bad news.<br>
<br>
Good news: Rx problem I believe is fixed. Return is true only if
we received something, else false. And the other interrupt
conditions are handled at the same time, so no hangs are seen when
restarting wifi. Rx overflow counter does increment properly.
Yea! Code has been pushed to my clone on Github.<br>
<br>
Bad news: I am still able to hang the bus, but I think it's on
the transmit side. The obd2ecu process can send up to 3 frames
back to back to report the ECU Name, followed soon after by
several more with to grab the VIN. Without any flow control on
the transmit side, and with a half-duplex CAN bus, that's just too
much. Turning off the VIN reporting (config set obd2ecu private
yes) seems to let everything run because I don't respond to the
VIN request (which lets everything drain as OBDWiz times out).
Also verified by putting temporary delays in the obd2ecu code to
let things drain a bit between frames. So, the transmit side is
still a bit fragile, depending on timing. Not sure quite what to
do here, as there is no easy place to queue things... Do we need
to go back to the old way with a delay in the obd2ecu code
(perhaps better than in the driver, no?). Architecturally it's
ugly, but this only occurs at startup, and I don't mind the
kludge. Do any other uses of the MCP busses do a burst of
transmitting? If not, I'll put the delays in the obd2ecu code and
call it close enough. Lemme know.<br>
<br>
For receive, I'd go with what I have for now, if Michael would be
so kind as to review what I have done first.
<a class="moz-txt-link-freetext"
href="https://github.com/bitsofgreg/Open-Vehicle-Monitoring-System-3/blob/master/vehicle/OVMS.V3/components/mcp2515/mcp2515.cpp"
moz-do-not-send="true">https://github.com/bitsofgreg/Open-Vehicle-Monitoring-System-3/blob/master/vehicle/OVMS.V3/components/mcp2515/mcp2515.cpp</a>
Hopefully he'll be back on line before I get up in the morning.
Wonderful how the Earth's spin helps with the teamwork.<br>
<br>
I'll keep poking at things tonight, and take it out for a spin in
the car tomorrow, just to see everything working together. But as
it is now, it's much better than it was before. Really, this
time. :)<br>
<br>
Greg<br>
<br>
<br>
<div class="moz-cite-prefix">Greg D. wrote:<br>
</div>
<blockquote type="cite"
cite="mid:e1ebf1b2-15e2-fa7b-92e1-6ed2a4972b63@gmail.com">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
Hi Mark,<br>
<br>
I believe you are right about the multiple flags, and the code
only processing Rx and "error" separately. Fundamentally, a
roll-over from buffer 0 to buffer 1 isn't really an error, just
a statement of fact on what happened. So, we should have buffer
1 and the rollover flag at the same time, which in fact is what
I saw. I need to handle the Rx overflow at the same time as the
buffer 1 receive, I think...<br>
<br>
I need to grab some dinner, but have a fix in the works. Will
report back in a few hours, hopefully with good news...<br>
<br>
Greg<br>
<br>
<br>
<div class="moz-cite-prefix">Mark Webb-Johnson wrote:<br>
</div>
<blockquote type="cite"
cite="mid:E10D22FC-01E4-4976-8A90-EA916B9CE7F1@webb-johnson.net">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
<div class=""><br class="">
</div>
The design of the system is as follows:
<div class=""><br class="">
</div>
<div class="">
<ul class="MailOutline">
<li class="">The can object CAN_rxtask listens on the rx
queue to receive instructional messages from canbus
drivers. These can be:</li>
<ul class="">
<li class="">CAN_frame: simply passes an entire incoming
can frame to the IncomingFrame handler.</li>
<li class="">CAN_rxcallback: an instruction for the
CAN_rxtask to call the RxCallback task repeatedly.</li>
<li class="">CAN_txcallback: an instruction for the
CAN_rxtask to call the TxCallback once.</li>
</ul>
<li class="">In the case of CAN_rxcallback, the canbus
object RxCallback function is expected to return FALSE
to indicate nothing should be done and RxCallback should
not be called again, or TRUE to indicate an incoming
frame has been received and should be passed
to IncomingFrame.</li>
<li class="">The system is arranged so that individual bus
driver interrupt implementations can be fast and
efficient.</li>
<ul class="">
<li class="">The driver can choose to receive the frame
in the interrupt handler itself, and pass it with
CAN_frame to CAN_rxtask. The esp32 can driver uses
this option.</li>
<li class="">Or the driver can choose to delay the
reception of the frame to the RxCallback stage, and
merely pass an indication with CAN_rxcallback. The
mcp2515 driver uses this option.</li>
</ul>
<li class="">The true/false response from RxCallback is
designed to allow the callback to signal it received a
frame or not. If it received a frame, then it is called
again.</li>
<li class="">This approach is used in order to be able to
centralise the reception of CAN frames to one single
task (avoiding having to run individual tasks for each
canbus, hence saving stack RAM).</li>
</ul>
<div><br class="">
</div>
<div>The RxCallback should definitely ONLY return true if an
actual can message has been received, and is being passed
back in the frame pointer parameter.</div>
<div><br class="">
</div>
<div>I suspect the issue is that the mcp2515 RxCallback is
being faced with multiple error flags. Changing that to a
return true (as Greg has done) has the undesired
side-effect of issuing a spurious IncomingFrame (with
garbage/blank frame), but also causes the RxCallback to be
called again (clearing the error flag). Perhaps the
solution is to put a loop in RxCallback so that if an
error condition is found, it should be cleared, but then
loop again and keep clearing errors until no more are
found, then return false? I think that in the mcp2515
case, this error clearing loop can be simply handled in
the RxCallback itself.</div>
<div><br class="">
</div>
<div>The alternative is to change the RxCallback logic so
that the return bool value means simply ‘loop’ (call me
again, please), and have the RxCallback itself
call IncomingFrame(), rather than passing a frame as a
parameter. If Michael/Greg think this is a better
approach, I am happy to make that change - it is pretty
trivial.</div>
<div><br class="">
</div>
<div>Regards, Mark.</div>
<br>
</div>
</blockquote>
</blockquote>
</blockquote>
<br>
<pre class="moz-signature" cols="144">--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
</pre>
</body>
</html>