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@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@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@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev
_______________________________________________ OvmsDev mailing list OvmsDev@lists.openvehicles.com http://lists.openvehicles.com/mailman/listinfo/ovmsdev