BRT Community

Please login or register.

Login with username, password and session length
Advanced search  

News:

Welcome to the Bridgetek Community!

Please read our Welcome Note

Technical Support enquires
please contact the team
@ Bridgetek Support

Please refer to our website for detailed information on all our products - Bridgetek - Bridging Technology

Author Topic: API inconsistencies  (Read 5642 times)

Rudolph

  • Sr. Member
  • ****
  • Posts: 389
    • View Profile
API inconsistencies
« on: October 31, 2022, 11:17:01 AM »

As I am going thru the MISRA-C report for my library I am finding API inconsistencies.

CMD_BITMAP_TRANSFORM
The given C prototype has "uint16_t result" as last parameter.
While this by itself is not correct and "result" should either be the return value or a pointer,
the description for result is:
"result return. Set to -1 on success, or 0 if it is not possible to find the solution matrix."

Which one is it, uint16_t or int16_t?
And actually the native type for RAM_CMD would be uint32_t.
Logged

BRT Community

  • Administrator
  • Hero Member
  • *****
  • Posts: 732
    • View Profile
Re: API inconsistencies
« Reply #1 on: November 04, 2022, 12:19:21 PM »

Hello Rudolph,

Thank you for your post. The result parameter can be treated as either 32-bit or 16-bit:

The firmware always writes either 0x00000000 or 0xffffffff into the word.
Users can treat it as a 16-bit value, or (more conveniently) 32-bit. Either works fine.

Best regards BRT Community
Logged

Rudolph

  • Sr. Member
  • ****
  • Posts: 389
    • View Profile
Re: API inconsistencies
« Reply #2 on: November 07, 2022, 09:27:27 PM »

Well ok, that is -1 if you treat it like an int16_t.  :)
My point was though that the BT81x Programming Guide is using uint16_t result and is saying "Set to -1 on success".
So technically it either is int16_t or it returns 0xffff on success. :-)
And of course you do not get a value out of a function like the API is now.

Another issues I came across is with all the int16_t parameters that do not seem to make sense.
Like int16_t font in cmd_button, cmd_text, cmd_toggle, cmd_keys and cmd_number.
While this is for the BITMAP_HANDLE which has a valid range from 0 to 31.
16 bits is fine, making it 8 bit would require a pad-byte for the commands - but why signed?
And then we have cmd_romfont, cmd_setfont, cmd_setfont2 and cmd_fontcache which all use uint32_t font.

We have int16_t width and height options for cmd_button, cmd_keys, cmd_scrollbar, cmd_sketch, cmd_slider and cmd_progress - why not uint16_t?
cmd_toggle - int16_t width
cmd_clock, cmd_dial, cmd_gauge - int16_t radius

And cmd_calibratesub is using uint16_t for width and height.

So far I did not bother too much, I put in a cast and it worked.
But now that I started to do MISRA-C 2012 checks I had to put in quite a number of extra casts to tell the static code analyzer tool that
I really want to put two int16_t values in one uint32_t value.

This is what I meant by API inconsistencies, if there is no compelling reason to not do this the API in the programming guide would benefit from some cleanup,
make it unsigned wherever possible and wherever negative values are impossible.

I just tried this and it works:
EVE_cmd_gauge(400, 200, 100, 0, 3, 2, 100, 1000);
EVE_cmd_button(400, 300, 100, 100, 28, 0, "Hello!");

This however does not work not at all:
EVE_cmd_gauge(400, 200, -100, 0, 3, 2, 100, 1000);
EVE_cmd_button(400, 300, -100, 100, 28, 0, "Hello!");
EVE_cmd_button(400, 300, 100, -100, 28, 0, "Hello!");
EVE_cmd_button(400, 300, 100, 100, -28, 0, "Hello!");

With all these I get a black screen as if the co-processor is not happy about the commands.
And trying the cmd_button in EVE Screen Editor it actually tells me "Co-processor engine fault".

So why signed parameters when going negative is not an option?
Logged