[Ovmsdev] Proposed changes to IncomingPollReply and IncomingPollError - before making a pull request

Mark Webb-Johnson mark at webb-johnson.net
Fri Jun 2 14:07:29 HKT 2023


Seems to be:

IncomingPollReply: 16 -> 32 bytes
IncomingPollError: 10 -> 22 bytes

Using a pointer to a structure would bring that down to 4 bytes.

Given the single level of function call, I don’t think 10 to 32 bytes is excessive or a problem for the stack.

However, the advantage of using a structure is that in future if we don’t want to add things, we don’t need to change all the function prototypes of all the users. So long as the overhead of populating the structure is not excessive, that does seem a more future-proof way of doing things.

Regards, Mark.

> On 2 Jun 2023, at 12:46 PM, Michael Geddes <frog at bunyip.wheelycreek.net> wrote:
> 
> Hi all,
> 
> So I have some changes coming that depend heavily on this function signature change. What it is doing is taking the members of OvmsVehicle that are specific to the ISOTP (and VWTP) protocols and making the parameters to both IncomingPollReply and IncomingPollError.   
> 
> This way of doing it makes all the bits explicit parameters, which means a bunch more parameters passed on the stack.
> 
> Original:
> -    virtual void IncomingPollReply(canbus* bus, uint16_t type, uint16_t pid, const uint8_t* data, uint8_t length, uint16_t mlremain);
> -    virtual void IncomingPollError(canbus* bus, uint16_t type, uint16_t pid, uint16_t code);
> 
> New:
> virtual void IncomingPollReply(canbus* bus, uint32_t moduleidsent, uint32_t moduleid, uint16_t type, uint16_t pid, const uint8_t* data, uint16_t mloffset, uint8_t length, uint16_t mlremain, uint16_t mlframe, const OvmsPoller::poll_pid_t &pollentry);
> virtual void IncomingPollError(canbus* bus, uint32_t moduleidsent, uint32_t moduleid, uint16_t type, uint16_t pid, uint16_t code, const OvmsPoller::poll_pid_t &pollentry);
> 
> The other way of achieving the same result would be to create a struct that contains these elements, and then to pass that as a const reference.
> 
> Thoughts?  If I need to make modifications to the signatures, I'd prefer to do it before I make the pull request.
> 
> //.ichael 
> _______________________________________________
> OvmsDev mailing list
> OvmsDev at lists.openvehicles.com
> http://lists.openvehicles.com/mailman/listinfo/ovmsdev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvehicles.com/pipermail/ovmsdev/attachments/20230602/34b523e6/attachment.htm>


More information about the OvmsDev mailing list