[Ovmsdev] CAN-3 broken again?

Sat Jan 13 02:01:19 HKT 2018

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.
>
>> On 11 Jan 2018, at 8:42 PM, Michael Balzer <dexter at expeedo.de <mailto:dexter at expeedo.de>> wrote:
>>
>> 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
>>       o → 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:
>>>>>       o CAN_frame: simply passes an entire incoming can frame to the IncomingFrame handler.
>>>>>       o CAN_rxcallback: an instruction for the CAN_rxtask to call the RxCallback task repeatedly.
>>>>>       o 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.
>>>>>       o 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.
>>>>>       o 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 at lists.teslaclub.hk <mailto:OvmsDev at lists.teslaclub.hk>
>> http://lists.teslaclub.hk/mailman/listinfo/ovmsdev
>
>
>
> _______________________________________________
> OvmsDev mailing list
> OvmsDev at 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.teslaclub.hk/pipermail/ovmsdev/attachments/20180112/6001fc26/attachment.html>