[Ovmsdev] BMS generalization

[Mark Webb-Johnson]

Robin,

I am looking at your IncomingPollComplete, pull request, but it seems very confusing to me.

The current design is that OvmsVehicle::PollerReceive calls IncomingPollReply() in several places, to pass the result up to the vehicle code for decoding. It pretty much just passes the data part of the frame (after decoding the first part containing PID, result code, etc).

Now, you have IncomingPollComplete(). You have changed OvmsVehicleIncomingPollReply from pure virtual to now have code?

You have a ’static uint8_t *buf’ defined in your new IncomingPollReply, but I can’t see where that get freed. Also there only seems to be one, so what happens when multiple ‘ml’ style polls are defined? I’m really not sure if this should be a static - perhaps a vehicle object member variable would make more sense (Iike the other m_poll member variables, as this IncomingPollComplete is concerning m_poll_ml)?

I am really not sure what is required/solved here? The vehicle.{h,cpp} change is mixed in with changes to the nissan leaf vehicle code, in the same commits, which makes things harder to see. In future, could you make one commit to the base code, and one to the vehicle module - if they are related/dependent, they can be in the same pull request, but mixing everything together with commits changing commits, makes it hard to see what is going on (or the impact that the changes to base code could have on other vehicle modules).

Regards, Mark.

> On 22 Nov 2018, at 11:32 PM, Robin O'Leary <ovmsdev at caederus.org> wrote:
> 
> I have used this to add support for the Leaf.  Very cool!
> 
> I also added a new way to process incoming poll replies:
> 
> 	void OvmsVehicle::IncomingPollComplete(canbus* bus, uint16_t moduleid, uint16_t type, uint16_t pid, uint8_t* data, uint8_t length)
> 
> Instead of implementing IncomingPollReply() and handling each frame as
> it arrives, vehicles can instead implement IncomingPollComplete(), which
> gets called once after all the frames are concatenated into data[length].
> 
> Robin.
> _______________________________________________
> OvmsDev mailing list
> OvmsDev at lists.openvehicles.com
> http://lists.openvehicles.com/mailman/listinfo/ovmsdev