System Event Handler - Documentation Error?

Firmware
1

I have Argon code that has compiled and been working for 2 years now. In recent preparation for a product upgrade, I went to recompile the code under OS 3.2 and I got a compiler error. Ditto for OS 3.1. The code compiles and executes OK under OS 3.0. Here is the offending function (a system event handler):

void firmwareupdatehandler(system_event_t event, int data) {
    switch (data) {
    case firmware_update_begin:
        writeToLCD("Firmware update","in progress");
        digitalWrite(READY_LED,LOW);
        digitalWrite(ADMIT_LED,LOW);
        digitalWrite(REJECT_LED,HIGH);
        break;
    case firmware_update_complete:
        //writeToLCD("Firmeware update","complete");  // xxx this didn't get called
        break;
    case firmware_update_failed:
        //writeToLCD("Firmware update","failed");  // xxx this is called even on successful update??
        break;
    }
}

Here is the compiler error message:

src/checkinfirmware.ino: In function 'void firmwareupdatehandler(system_event_t, int)':
src/checkinfirmware.ino:260:10: error: narrowing conversion of 'firmware_update_failed' from 'unsigned int' to 'int' [-Wnarrowing]
  260 |     case firmware_update_failed:
      |          ^~~~~~~~~~~~~~~~~~~~~~

Obviously, changing int to unsigned int solved the problem and the code compiles OK under OS 3.1 and 3.2.

I am writing with 2 questions:

  1. The original argument list for the system event handler comes straight out of the Particle on-line documentation. Is this just a documentation error, or was there some change in the OS event publication between OS 3.0 and OS 3.1 that I should know about? More specifically, was there some erroneaus code change in OS 3.1 that might be changed back in the future (and hence I might get another compile issue)?

  2. I appears that OS 3.1 and above compiler checks are much stricter than in OS 3.0 and below. This isn’t necessarily bad, but it does mean that older code that was working fine may not be able to be upgraded to the latest OS release. I had a previous issue with a library that was an old port from Arduino and contained some non-harmful bugs, but will not compile under OS 3.1 and above, leaving me at OS 3.0 until Particle ports over the latest bug-fixed version of this library (or unless I use my own library port which I prefer not to do for obvious reasons). Is this stricter compiler setting intentional on Particle’s part? Does the user have the option for a less strict compile (without risking other issues with Particle’s toolchain)?

1 Like
2

You’re switching on data. You should be switching on event.

system_event_t is a 64-bit integer, as are all of the constant like firmware_update_failed so your error is because you’re comparing a 64-bit constant against a 32-bit value (data).

The warning level is higher in recent versions of Device OS. You could override it for local builds in Particle Workbench, but in this case it flagged a real error.

3

@rickkas7: The event handler is registered in setup() to fire off on a firmware update event. Here is the code for this:

System.on(firmware_update, firmwareupdatehandler);

The handler is processing the data in order to display an appropriate message to a user. The user does not otherwise know what is happening if the firmware is being updated remotely (cloud flashing, because the products are remotely deployed). That is why the switch() is on the data parameter. We need to know what the firmware update status is to display to the user.

According to the error message from the compiler, the compile fails because the calling code sends an unsigned int (not a long) to the event handler, but the event handler data parameter is defined as an int. The compiler sees this as a downgrade in precision and due to stricter rules, it fails the compile.

Here is the prototype for an event handler, right out of the Particle documentation:

void handler(system_event_t event, int data);

The data type for “data” is defined in the Particle documentation to be an int, but the compiler says it requires an unsigned int. The Particle documentation does not reflect what the compiler is requiring; that’s a fact. My question is whether this is simply a documentation error or whether the OS code is designed for something else and there is a bug. If the latter, the bug might be corrected later causing further problems in future upgrades. If this is simply a documentation error, then my change to an unsigned int would be correct for now and for the future.

5

OK, now I see what you’re saying. That’s a bug in the Device OS include file.

enum SystemEventsParam {
    //
    network_credentials_added = 1,
    network_credentials_cleared = 2,
    firmware_update_failed = (uint32_t)-1,
    firmware_update_begin = 0,
    firmware_update_complete = 1,
    firmware_update_progress = 2,
    // Network status. bit 0 is clear if it's a transition state (e.g. powering up, connecting), is set when it's a rest state (connected/ready etc)
    network_status_powering_off     = 1<<1 | 0,
    network_status_off              = 1<<1 | 1,
...

The SystemEventsParam enum should probably be type-specified as an int, because the handler is declared as:

std::function<void(system_event_t, int)> handler

The type of data is correct, but you can work around this by casting data to a (SystemEventsParam) in the switch for now.

6

@rickkas7: Thanks, I understand this now and it is good to know that there will be some bug fix in the future (to be on the lookout for if a future compile fails). If I understand you correctly, the firware_update_xxxx items will be removed from the SystemEventsParam enum and placed into their own enum or perhaps a bunch of #define? It is just strange to see a value of -1 cast to an unsigned 32 bit int, but I guess that someone at Particle took a quick workaround to having a negative value in an enum :smile:

7

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.