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