Skip to content

Conversation

@soundanalogous
Copy link
Member

This update frees 122 bytes of RAM. I have replace the old string error messages in StandardFirmata with single byte error codes (0 - 127). A user can add custom error codes to any Firmata sketch. The client library should provide a message for each error code (or otherwise handle the error as necessary).

This update may impact any client library that handles the old string error messages sent by StandardFirmata (if this is actually the case with any client library - I suspect it is rare). However, given that this pull request is for Firmata 2.4.0 and the RAM savings and overall more explicit nature of the new error_code I think this update justifies the backwards incompatible change.

@SolidSoils
Copy link

Adding an error message to the Firmata protocol is a good idea.

For now, the proposed message format only supports 1 data byte, thus offering codes ranging from 0 - 127.
Why not use two bytes? It would provide a bit more flexibility and at the same time would be more consistent with most other Firmata messages.

0  START_SYSEX
1  ERROR_MSG    (0x68)
2  error code 0 (LSB)
3  error code 1 (MSB)
3  END_SYSEX

E.g, bigger error codes could make it easier to 'classify' error codes, by reserving on or more bits for that purpose.

@soundanalogous
Copy link
Member Author

I agree. I'll update the proposal and pull request. Bigger thing I still need to get consensus on here is moving error messages from Strings (as they are currently represented... taking up a lot of SRAM) to numeric codes. It will be a backwards incompatible update for anyone who was relying on the error string messages.

@SolidSoils
Copy link

I would like to share one more thought: reserve error codes 0 to 127 for the Firmata protocol itself. Doing so you keep the possibility to implement standardized Firmata error codes later on.
E.g: 0x01 - pin does not exist or 0x02 - invalid pin mode

@ntruchsess
Copy link
Member

Error msg might need a bit of context (e.g. Feature on Pin didn't succeed - this requires information about pinmode and pin). So I'd suggest:

0  START_SYSEX
1  ERROR_MSG    (0x68)
2  pin mode (agree on e.g. 7f for not pinmode specific error messages?)
3  pin
4  pin mode specific error code 0 (LSB)
5  pin mode specific error code 1 (MSB)
6  (optional aditional pin mode specific bytes)
3  END_SYSEX

@SolidSoils
Copy link

I prefer to keep the error message simple and basic and avoid to tie it up to specific scenarios (like pinmode). The bytecodes of my example regarding a pinmode error would simply look like this:

0 0xF0 (START_SYSEX)
1 0x68 (ERROR_MSG)
2 0x01
3 0x00
4 0xF7 (END_SYSEX)

When there is a need for communicating more data (i.e. pin number and mode), it is better to append it after the initial error code:

0 0xF0 (START_SYSEX)
1 0x68 (ERROR_MSG)
2 0x01 (ERROR_CODE LSB)
3 0x00 (ERROR_CODE MSB)
4 0x01 (DATA_BYTE1)
5 0x01 (DATA_BYTE2)
6 ... (add bytes as required)
- 0xF7 (END_SYSEX)

@ntruchsess
Copy link
Member

In ConfigurableFirmata you'd have two cases. The error either comes from the Firmata base or from one of the configured features which are (with the exceptions of FirmataExt and FirmataScheduler) associated with a pinmode (which is not nessesary a mode the arduino allows to set nativly, but mere a numer that identifies the feature configured for this pin). Thats why I don't think the pinmode is optional. If it were all error-messages coming from a feature would contain two error codes: First the code that tells its 'not core Firmata', then the one that is specific for this feature. The only way to overcome this would be some kind of 'error-code-registry' for custom-features which I'd like to avoid.

  • Norbert

@soundanalogous
Copy link
Member Author

What about:

0 START_SYSEX
1 ERROR_MSG       0x68
2 ERROR_TYPE (corresponds to configurable feature, memory error, general error, etc)
3 ERROR_CODE LSB
4 ERROR_CODE MSB (may be able to get away with a single byte if we have message types)
5 DATA_BYTE1 (supplementary data)
6 DATA_BYTE2
7 ... (add bytes as required)
- END_SYSEX

Error type is similar to what Norbert is talking about regarding pin modes. So you could have feature-specific errors. I2C for example (where most of the current string error messages are). I also need to add errors for memory usage since we have dynamic allocation so that's another broad category. However memory errors are also feature related (Servo, Stepper and OneWire feature classes all allocate memory dynamically via malloc).

@ntruchsess
Copy link
Member

isn't this basically the same as my proposal without the mandatory pin (indeed I intended the pin to be optional)? Or would you rather put the pinmode into byte 3 if ERROR_TYPE says 'pinmode-specific-error'? I don't have a strong oppinion here - it just should be short in terms of memory required to send the message, but not too short to not restrict introduction of future pinmodes (aka 'features' in configurable).

@SolidSoils
Copy link

I have been thinking about the error type byte and I am still not convinced it would make sense to make it part of the error message. It would require a well thought out taxonomy of error types, describing the meaning of all supported error types precisely.

It is easier to reserve the first 127 (or 1000 or whatever) codes for the Firmata protocol and leave it up to the user to design error codes of their own. The reserved error codes pretty much tell you what type of error you are dealing with.

Error code examples:

  • 0 - OK
  • 1 - Setup error
  • 2 - Memory error
  • 3 - Pinmode error

etc.

@ntruchsess
Copy link
Member

How would you classify errors from 1-Wire, I2C, Servo, Shift or any other (not yet developed) feature? By using a byte for error_type/pinmode/feature the developer of a new feature has to care about getting a pinmode assigned and is free to choose his errorcodes as he likes without getting in conflict with other features. As a user (not developer) of custom-features you wouldn't want to go to the sourcecode and care about uniqueness of error-codes.

@soundanalogous
Copy link
Member Author

I agree with Norbert that we need some level of classification. For example there are currently 4 i2c errors defined in StandardFirmata:

"I2C Read Error: Too many bytes received"
"I2C Read Error: Too few bytes received"
"10-bit addressing mode is not yet supported"
"too many queries"

Each error message is currently sent as a Firmata String message. Having a pin or error type of I2C for example would make it easier to keep error codes organized. If we shift to a numeric (rather than String) error message implementation, it will be the responsibility of the Firmata client implementation to present any errors to the user in a meaningful way based on the numberic codes received from the board.

So the first i2c error message above could be represented like this using numeric codes:

0 START_SYSEX
1 ERROR_MSG       0x68
2 ERROR_TYPE      0x06  (i2c pin mode)
3 TOO_MANY_BYTES  (0x00) // enumerate
...

This feature / pin specific features are organized and IMO would be much easier to keep organized on the client side. If at some point in the future you need a new I2C error type, you give it the next value in the enumeration. Contrast that with a single list of numeric error codes that are sequential. You could have feature specific error codes scattered throughout list.

5    TOO_MANY_BYTES
6    TOO_FEW_BYTES
21   SOME_OTHER_I2C_ERROR
187 YET_ANOTHER _I2C_ERROR

@SolidSoils
Copy link

OK, when we decide to introduce an error type (or class), it would be necessary to dictate a fixed set of well defined error types, wouldn't it? I mean, when you leave it up to implementers of the protocol to design error types themselves, client code can not rely on the meaning of the error type it receives, thus rendering this code useless.

E.g., error types could be (only to draw the picture):

  1. Setup
  2. I2C
  3. Wire
  4. Servo
    ...
  5. Custom

What's your opinion on this?

@soundanalogous
Copy link
Member Author

Feature level types are already defined as pin modes (this is what Norbert has been referring to):

#define ANALOG                  0x02 // analog pin in analogInput mode
#define PWM                     0x03 // digital pin in PWM output mode
#define SERVO                   0x04 // digital pin in Servo output mode
#define SHIFT                   0x05 // shiftIn/shiftOut mode
#define I2C                     0x06 // pin included in I2C setup
#define ONEWIRE                 0x07 // pin configured for 1-wire
#define STEPPER                 0x08 // pin configured for stepper motor

When a new feature is added, a new pin mode is added and thus a new error type (and each error type can have multiple type specific error codes) can be added. If there is no SPI support now there can be no SPI errors until a SPI feature is added for example. Pin types would need to be reserved up to a value of 0x127. There should also be a section of numbers reserved for user application error types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants