Part one (TX queue) done & pushed.
OVMS > can can1 status
CAN: can1
Mode: Active
Speed: 500000
Rx pkt:
236657
Rx
err: 1
Rx
ovrflw: 0
Tx pkt:
106378
Tx
delays: 4
Tx
err: 0
Tx
ovrflw: 0
Err flags: 0x800caa
TX performance is rock steady on can1 -- the
delays occurred when sending the stop charge
request (as expected). I can't test can2/3,
Greg & Geir, could you…?
The TxCallback can't be used on the mcp2515.
The ISR can't query the IRQ register, so the
TX IRQs are now also handled by the
RxCallback(). As the TX IRQs need to be
cleared before loading the next frame, this
needs another SPI call. I hope that doesn't
introduce new problems.
No changes are necessary to the application
code (well, except you can remove any hard
coded delays now). The TX queue has a length
of 20 frames and will automatically be used
by the drivers when no TX buffers are free.
If an application wants to know whether a
frame was sent immediately or gets delayed
it can check the return code of the Write()
method. Write() now also can take a second
parameter for the maximum wait time for
space in the TX queue to become available if
it's full (default 0 = fail immediately if
queue is full).
I also added logging of CAN errors. It's
currently activated by "can … trace on", I
don't think this needs to be active by
default, just for CAN issue debugging.
E (45718) can: Error can1
rxpkt=3 txpkt=0 errflags=0x800caa rxerr=1
txerr=0 rxovr=0 txovr=0 txdelay=0
E (83528) can: Error can1
rxpkt=7483 txpkt=226 errflags=0x800caa
rxerr=1 txerr=0 rxovr=0 txovr=0 txdelay=0
…that's also a first part of the logging
extension (part two).
Regards,
Michael
Am 12.01.2018
um 19:01 schrieb Michael Balzer:
Yes, I had something like that in mind. On
TX IRQ, the drivers send CAN_txcallbacks
to the CAN_rxtask. The CAN_rxtask then
fetches frames from the TX queue and calls
the TxCallback until all TX buffers of the
driver are full. From the already existing
TxCallback() stubs I suppose you had
planned a scheme like that already? ;)
Greg, can you create a pull request for
your MCP2515 change? I'd like to merge
that before beginning on the drivers.
Thanks,
Michael
Am 12.01.2018
um 01:19 schrieb Mark Webb-Johnson:
Option B sounds like a good approach.
Presumably we are just
polling the tx queue in the existing
CAN_rxtask based on TxCallback?
Regards, Mark.
Greg,
Mark,
I can check your new code
after work.
For the TX
performance/overflow issue,
there are basically two
options:
- 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)
- 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
- → 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
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.
I can try to implement that
this weekend if it's urgent
now.
Regards,
Michael
Am 11.01.2018 um 05:55
schrieb Greg D.:
Hi Mark, Micheal,
Ok, good news and bad news.
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.
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.
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. https://github.com/bitsofgreg/Open-Vehicle-Monitoring-System-3/blob/master/vehicle/OVMS.V3/components/mcp2515/mcp2515.cpp
Hopefully he'll be back on
line before I get up in the
morning. Wonderful how the
Earth's spin helps with the
teamwork.
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. :)
Greg
Greg
D. wrote:
Hi Mark,
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...
I need to grab some
dinner, but have a fix in
the works. Will report
back in a few hours,
hopefully with good
news...
Greg
Mark
Webb-Johnson wrote:
The design of the system
is as follows:
- The can
object CAN_rxtask
listens on the rx
queue to receive
instructional
messages from
canbus drivers.
These can be:
- CAN_frame:
simply passes an
entire incoming
can frame to the
IncomingFrame
handler.
- CAN_rxcallback:
an instruction
for the
CAN_rxtask to
call
the RxCallback
task repeatedly.
- CAN_txcallback:
an instruction
for the
CAN_rxtask to
call
the TxCallback
once.
- 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.
- The
system is arranged
so that individual
bus driver
interrupt
implementations
can be fast and
efficient.
- 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.
- 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.
- 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.
- 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).
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.
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.
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.
Regards,
Mark.
--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________
OvmsDev mailing list
OvmsDev@lists.teslaclub.hk
http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
_______________________________________________
OvmsDev mailing list
OvmsDev@lists.teslaclub.hk
http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
_______________________________________________
OvmsDev mailing list
OvmsDev@lists.teslaclub.hk
http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
--
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26