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

[Michael Geddes]

Yeah, that makes sense about future proofing.

Even if I have the struct on the stack it should be efficient enough to
populate.

I might even consider using the structure to contain those values on the
class itself making it zero cost to populate. That might be a later to do.

Michael

On Fri, 2 June 2023, 2:07 pm Mark Webb-Johnson, <mark at webb-johnson.net>
wrote:

> 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
>
>
> _______________________________________________
> 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/c1c066b8/attachment.htm>