<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Option B sounds like a good approach.<div class=""><br class=""></div><div class="">Presumably we are just polling the tx queue in the existing CAN_rxtask based on TxCallback?</div><div class=""><br class=""></div><div class="">Regards, Mark.<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On 11 Jan 2018, at 8:42 PM, Michael Balzer <<a href="mailto:dexter@expeedo.de" class="">dexter@expeedo.de</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
  
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" class="">
  
  <div text="#000000" bgcolor="#FFFFFF" class="">
    <div class="moz-cite-prefix">Greg, Mark,<br class="">
      <br class="">
      I can check your new code after work.<br class="">
      <br class="">
      For the TX performance/overflow issue, there are basically two
      options:<br class="">
      <ul class="">
        <li class="">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 class="">
        </li>
        <li class="">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 class="">
          <li class="">→ 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 class="">
          </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 class="">
      <br class="">
      I can try to implement that this weekend if it's urgent now.<br class="">
      <br class="">
      Regards,<br class="">
      Michael<br class="">
      <br class="">
      <br class="">
      Am 11.01.2018 um 05:55 schrieb Greg D.:<br class="">
    </div>
    <blockquote type="cite" cite="mid:b229b09f-ef72-c564-7839-fa8a815157cd@gmail.com" class="">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" class="">
      Hi Mark, Micheal,<br class="">
      <br class="">
      Ok, good news and bad news.<br class="">
      <br class="">
      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 class="">
      <br class="">
      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 class="">
      <br class="">
      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 class="">
      <br class="">
      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 class="">
      <br class="">
      Greg<br class="">
      <br class="">
      <br class="">
      <div class="moz-cite-prefix">Greg D. wrote:<br class="">
      </div>
      <blockquote type="cite" cite="mid:e1ebf1b2-15e2-fa7b-92e1-6ed2a4972b63@gmail.com" class="">
        <meta http-equiv="Content-Type" content="text/html;
          charset=UTF-8" class="">
        Hi Mark,<br class="">
        <br class="">
        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 class="">
        <br class="">
        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 class="">
        <br class="">
        Greg<br class="">
        <br class="">
        <br class="">
        <div class="moz-cite-prefix">Mark Webb-Johnson wrote:<br class="">
        </div>
        <blockquote type="cite" cite="mid:E10D22FC-01E4-4976-8A90-EA916B9CE7F1@webb-johnson.net" class="">
          <meta http-equiv="Content-Type" content="text/html;
            charset=UTF-8" class="">
          <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 class=""><br class="">
            </div>
            <div class="">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 class=""><br class="">
            </div>
            <div class="">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 class=""><br class="">
            </div>
            <div class="">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 class=""><br class="">
            </div>
            <div class="">Regards, Mark.</div>
            <br class="">
          </div>
        </blockquote>
      </blockquote>
    </blockquote>
    <br class="">
    <pre class="moz-signature" cols="144">-- 
Michael Balzer * Helkenberger Weg 9 * D-58256 Ennepetal
Fon 02333 / 833 5735 * Handy 0176 / 206 989 26
</pre>
  </div>

_______________________________________________<br class="">OvmsDev mailing list<br class=""><a href="mailto:OvmsDev@lists.teslaclub.hk" class="">OvmsDev@lists.teslaclub.hk</a><br class="">http://lists.teslaclub.hk/mailman/listinfo/ovmsdev<br class=""></div></blockquote></div><br class=""></div></body></html>