NRF: `getAddress(1)` returns the current BLE address

[bobrippling]

This allows callers of NRF.getAddress() to get the current address, for example when setAddress() has been called - such as for Open Haystack

[bobrippling]

I suppose there's no way to check if this feature exists (except firmware version), perhaps a separate function would be better

[bobrippling]

Tangentially related to #2401 / #2506

[gfwilliams]

I think this is ok (same function) - you can always check firmware version.

However I think it still needs work - it looks like if you call getAddress(1) when you haven't set an address, it returns undefined, when it should fall through and return the default address?

[bobrippling]

Good spot, thanks - updated the PR

[gfwilliams]

Thanks! Looks good - ideally I'd use bool current and set the type to bool in the JSON but I'll just merge this now and make the change myself.

[gfwilliams]

Just FYI, if you search for jswrap_ble_getAddress you'll see it is used in a bunch of places that weren't updated, so it's getting called with random values for current. I've just put in a fix for it I hope.

I'm honestly not sure why the build didn't fail - maybe I need to look at making warnings about calling functions with incorrect parameters into full on errors.

[bobrippling]

Just FYI, if you search for jswrap_ble_getAddress you'll see it is used in a bunch of places that weren't updated, so it's getting called with random values for current. I've just put in a fix for it I hope.

I'm honestly not sure why the build didn't fail - maybe I need to look at making warnings about calling functions with incorrect parameters into full on errors.

Ah yeah, I was relying on the compiler to catch invalid calls to jswrap_ble_getAddress, hence why I missed all those. The reason the build didn't fail is a C thing - this would've caught it when it was a no-argument function:

-JsVar *jswrap_ble_getAddress();
+JsVar *jswrap_ble_getAddress(void);

There may be similar bugs, to catch them you can use -Wstrict-prototypes or change all the declarations and see what errors arise.

I've done a quick check and jswrap_graphics_theme appears - there may be more in other files but make isn't invoked with -k so it stops at the first error.

libs/graphics/jswrap_graphics.c:4575:8: error: conflicting types for 'jswrap_graphics_theme'; have 'JsVar *(JsVar *)' {aka 'struct JsVarStruct *(struct JsVarStruct *)'}
 4575 | JsVar *jswrap_graphics_theme(JsVar *parent) {
      |        ^~~~~~~~~~~~~~~~~~~~~
In file included from libs/graphics/jswrap_graphics.c:16:
libs/graphics/jswrap_graphics.h:94:8: note: previous declaration of 'jswrap_graphics_theme' with type 'JsVar *(void)' {aka 'struct JsVarStruct *(void)'}
   94 | JsVar *jswrap_graphics_theme(void);
      |        ^~~~~~~~~~~~~~~~~~~~~

---

If you were to change to (void), on some architectures you'd save an instruction too - ABIs such as x86 specify that calls to variadic or unspecified-parameter functions must load the number of floating point arguments into a register (usually zero, but it's still an instruction to load that).

[gfwilliams]

Thanks! That's interesting - I always assumed (void) vs () was effectively the same!

I've just fixed jswrap_graphics_theme.

I'd prefer not to have to change to (void) everywhere if there's a choice - it just seems like it'll be a pretty huge PR... although looking at https://github.com/espruino/Espruino/compare/master...bobrippling:Espruino:fix/unspec-args you only had to actually change the header files?

Don't suppose you know if there's a compiler switch to enable the behaviour without having to change it?

[bobrippling]

Yeah you've got two options really:

1. Compile the codebase as C23 (-std=c2x), to gain this:

   Functions with no arguments listed in the prototype void foo() are understood as taking no arguments (see removal of K&R function declarations)

The downsides are that there may be other things which need fixing to align with the standard upgrade, but given that the codebase is already C11 (or the GNU variant), I don't think it's a real issue: https://github.com/espruino/Espruino/blob/49b8ac8e97efaac9f562ad879e7cf3f927a57dd8/Makefile#L238-L238

And this option is very cheap to try out.

Or:

2. Get gcc to point out all the problematic declarations with -Werror=strict-prototypes. This will still require changing of the prototypes themselves, but it's only the header files that need to change, except for file-local/static functions