canopen-stack icon indicating copy to clipboard operation
canopen-stack copied to clipboard

Discussion on possible code size reductions

Open jernejsk opened this issue 3 years ago • 8 comments

As far as I can see, there are only 2 functions, which set offsets and in all cases, it's set to 0. What is rationale behind this control? Is it remnant of the past or for some future functionality? Removing it would lower code size and simplify stack a bit. While this probably doesn't affect big MCUs much, it can be noticeable on smaller MCUs, like the one I'm working on (proprietary design).

jernejsk avatar Jul 07 '22 08:07 jernejsk

This function code was used in some special cases a few years ago. Removing it is possible, but will break all existing user-written control functions.

As we are working on an update for 4.2.x, we could announce this change for the next major version 4.3 (with the SDO client segmented transfer). I think we can assume it is a relict and can remove it to gain some resources.

The breaking change will be:

/*!< Control type function prototype (OLD) */
typedef CO_ERR   (*CO_OBJ_CTRL_FUNC) (struct CO_OBJ_T *, struct CO_NODE_T *, uint16_t, uint32_t);

/* will be replaced by */

/*!< Reset type function prototype (NEW) */
typedef CO_ERR   (*CO_OBJ_RESET_FUNC) (struct CO_OBJ_T *, struct CO_NODE_T *);

michael-hillmann avatar Jul 12 '22 08:07 michael-hillmann

Ctrl handler is also used by COTAsync but it's scope is limited to PDOs, so ctrl handler might still be needed.

Please correct me if I'm wrong, but there are actually two ways that stack signals to reset offset:

  1. via ctrl handler, where function argument is set to CO_CTRL_SET_OFF and value to 0
  2. via read and write handler, where length parameter is set to 0

Second option is nice and doesn't need any additional handler. What do you think?

jernejsk avatar Jul 12 '22 08:07 jernejsk

You are right, it is used in asynchronous PDOs, too.

Summary:

  • TPDO: The obj->Type->Ctrl() is used to trigger the transmission when the PDO mappable object entry is changed during writing; function-code := always CO_TPDO_ASYNC, para := unused
  • OBJS: The obj->Type->Ctrl() is used to reset the internal working offset when reading/writing large object entries (domains, strings, parameters, etc.); function-code: always CO_CTRL_SET_OFF, para := new offset (allways set to 0)

On one hand: We may remove the two arguments (function-code and parameter) to eliminate the code required to pass these values into the function during calls. This removes some flexibility for new types we may write as part of the stack in the future. In my opinion, this is acceptable when you can confirm that the code size on your target is reduced after this change.

On the other hand: Removing this function completely removes the ability to add type-specific callback functions before each read/write operation and after a changing write operation. This will eliminate the TPDO feature "autonomous PDO transmission after changed object entry". This is not acceptable until we find an alternative approach to define object-entry-specific callbacks before/after read/write and after object-entry value changes.

Will removing the arguments in the control function solve your code size issue?

michael-hillmann avatar Jul 12 '22 09:07 michael-hillmann

Removing just parameter saves 136 B of code in my firmware, so not much, but it something. I think I'll get better code size savings by reworking some other parts of the firmware. Thanks for looking into this!

jernejsk avatar Jul 12 '22 09:07 jernejsk

Can you share your current size and the size you want to achieve? Just to get a feeling on the challenge ;-)

michael-hillmann avatar Jul 12 '22 09:07 michael-hillmann

Current firmware size without modifications is 65474 B and 65336 B without parameter in ctrl handler. Code space is 16 B less than 64 kB. CANopen stack with dictionary and custom types takes more than half of that. I need a few kBs of space to implement remaining features. There are some reserves by using size optimization level in compiler and some minor things across my firmware, which should be enough. I was just looking for low hanging fruits, like offset management here.

jernejsk avatar Jul 12 '22 10:07 jernejsk

One idea may be to add a conditional compilation flag USE_SDO_BLOCK (with defaults to 1) in co_cfg.h. When setting this to 0 e.g. via compile switch -D USE_SDO_BLOCK=0, the SDO block transfer is removed from the stack. Is this a possible enhancement for the stack in your project?

Note: this is not implemented right now and needs to be done. But I guess it is fairly easy to do.

michael-hillmann avatar Jul 12 '22 11:07 michael-hillmann

Thanks for reminding me. While block transfers are useful for speedy firmware updates, they are not essential. I can disable them as a last resort, if there won't be enough space.

jernejsk avatar Jul 12 '22 11:07 jernejsk