Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally tag CHIP_ERROR with source location #8340

Closed
kpschoedel opened this issue Jul 13, 2021 · 0 comments · Fixed by #8470
Closed

Optionally tag CHIP_ERROR with source location #8340

kpschoedel opened this issue Jul 13, 2021 · 0 comments · Fixed by #8470
Assignees

Comments

@kpschoedel
Copy link
Contributor

Problem

It can be difficult to identify the specific code that returned an error.

Proposed Solution

Convert CHIP_ERROR to a class that can (configurably) record the source location of an error, by including FILE and LINE in the …ERROR… constant macro expansions.

When not configured, this change must not increase the size of CHIP_ERROR or associated code.

@kpschoedel kpschoedel self-assigned this Jul 13, 2021
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 14, 2021
#### Problem

Having `CHIP_ERROR` be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Added a transitional configuration option to select whether
  `CHIP_ERROR` is an integer (as now) or a class type. Initially, only
  the Linux platform is configured with a class. The intent is to
  incrementally fix integer assumptions in other platforms, and finally
  remove the configuration flag and streamline the `ChipError` class.

- Since logging calls had to modified anyway, added `FormatError()`,
  which can be configured to either return an integer value or a string
  (via `ErrorStr()`).

- Fixed some actual type errors, where non-CHIP_ERROR values were
  treated as CHIP_ERRORs. (None appear to have had more serious
  consequences than a nonsense message from ErrorStr().)

#### Testing

- Modified affected unit tests.
- Sanity check using chip-tool.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 14, 2021
#### Problem

Having `CHIP_ERROR` be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Added a transitional configuration option to select whether
  `CHIP_ERROR` is an integer (as now) or a class type. Initially, only
  the Linux platform is configured with a class. The intent is to
  incrementally fix integer assumptions in other platforms, and finally
  remove the configuration flag and streamline the `ChipError` class.

- Since logging calls had to modified anyway, added `FormatError()`,
  which can be configured to either return an integer value or a string
  (via `ErrorStr()`).

- Fixed some actual type errors, where non-CHIP_ERROR values were
  treated as CHIP_ERRORs. (None appear to have had more serious
  consequences than a nonsense message from ErrorStr().)

#### Testing

- Modified affected unit tests.
- Sanity check using chip-tool.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 14, 2021
#### Problem

Having `CHIP_ERROR` be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Added a transitional configuration option to select whether
  `CHIP_ERROR` is an integer (as now) or a class type. Initially, only
  the Linux platform is configured with a class. The intent is to
  incrementally fix integer assumptions in other platforms, and finally
  remove the configuration flag and streamline the `ChipError` class.

- Since logging calls had to modified anyway, added `FormatError()`,
  which can be configured to either return an integer value or a string
  (via `ErrorStr()`).

- Fixed some actual type errors, where non-CHIP_ERROR values were
  treated as CHIP_ERRORs. (None appear to have had more serious
  consequences than a nonsense message from ErrorStr().)

#### Testing

- Modified affected unit tests.
- Sanity check using chip-tool.
mspang pushed a commit that referenced this issue Jul 15, 2021
* Make CHIP_ERROR a class type on Linux

#### Problem

Having `CHIP_ERROR` be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue #8340).

#### Change overview

- Added a transitional configuration option to select whether
  `CHIP_ERROR` is an integer (as now) or a class type. Initially, only
  the Linux platform is configured with a class. The intent is to
  incrementally fix integer assumptions in other platforms, and finally
  remove the configuration flag and streamline the `ChipError` class.

- Since logging calls had to modified anyway, added `FormatError()`,
  which can be configured to either return an integer value or a string
  (via `ErrorStr()`).

- Fixed some actual type errors, where non-CHIP_ERROR values were
  treated as CHIP_ERRORs. (None appear to have had more serious
  consequences than a nonsense message from ErrorStr().)

#### Testing

- Modified affected unit tests.
- Sanity check using chip-tool.

* review

* review

* zap regen
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 15, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on cc13x2_26x2, EFR32, QPG.

- Remove `operator bool()` (requested in project-chip#8383 review).

#### Testing

Existing tests should confirm no change to functionality.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 15, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on cc13x2_26x2, EFR32, QPG.

- Remove `operator bool()` (requested in project-chip#8383 review).

#### Testing

Existing tests should confirm no change to functionality.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 15, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on cc13x2_26x2, EFR32, QPG.

- Remove `operator bool()` (requested in project-chip#8383 review).

#### Testing

Existing tests should confirm no change to functionality.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 16, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on ESP32.

#### Testing

Passes esp32_qemu_tests.sh
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 16, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on ESP32.

#### Testing

Passes esp32_qemu_tests.sh
bzbarsky-apple pushed a commit that referenced this issue Jul 16, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue #8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on cc13x2_26x2, EFR32, QPG.

- Remove `operator bool()` (requested in #8383 review).

#### Testing

Existing tests should confirm no change to functionality.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 17, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on ESP32.

#### Testing

Passes esp32_qemu_tests.sh
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 17, 2021
#### Problem

It can be difficult to identify the specific code that returned an error.

#### Change overview

Added a configuration option, `CHIP_CONFIG_ERROR_SOURCE`. When this is
enabled (and assuming `CHIP_ERROR` is a class, which should eventually
be true for all builds), `…_ERROR_…` constant macros record the source
`__FILE__` and `__LINE__` where they are used, in the `CHIP_ERROR` object.

Unless `CHIP_CONFIG_SHORT_ERROR_STR` is configured, the source file and
line appear in the result of `ErrorStr()`.

Fixes project-chip#8340 _Optionally tag CHIP_ERROR with source location_

#### Testing

- revised `src/lib/support/tests/TestErrorStr.cpp`
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 19, 2021
#### Problem

It can be difficult to identify the specific code that returned an error.

#### Change overview

Added a configuration option, `CHIP_CONFIG_ERROR_SOURCE`. When this is
enabled (and assuming `CHIP_ERROR` is a class, which should eventually
be true for all builds), `…_ERROR_…` constant macros record the source
`__FILE__` and `__LINE__` where they are used, in the `CHIP_ERROR` object.

Unless `CHIP_CONFIG_SHORT_ERROR_STR` is configured, the source file and
line appear in the result of `ErrorStr()`.

Fixes project-chip#8340 _Optionally tag CHIP_ERROR with source location_

#### Testing

- revised `src/lib/support/tests/TestErrorStr.cpp`
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 19, 2021
#### Problem

It can be difficult to identify the specific code that returned an error.

#### Change overview

Added a configuration option, `CHIP_CONFIG_ERROR_SOURCE`. When this is
enabled (and assuming `CHIP_ERROR` is a class, which should eventually
be true for all builds), `…_ERROR_…` constant macros record the source
`__FILE__` and `__LINE__` where they are used, in the `CHIP_ERROR` object.

Unless `CHIP_CONFIG_SHORT_ERROR_STR` is configured, the source file and
line appear in the result of `ErrorStr()`.

Fixes project-chip#8340 _Optionally tag CHIP_ERROR with source location_

#### Testing

- revised `src/lib/support/tests/TestErrorStr.cpp`
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 19, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on ESP32.

#### Testing

Passes esp32_qemu_tests.sh
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 19, 2021
#### Problem

It can be difficult to identify the specific code that returned an error.

#### Change overview

Added a configuration option, `CHIP_CONFIG_ERROR_SOURCE`. When this is
enabled (and assuming `CHIP_ERROR` is a class, which should eventually
be true for all builds), `…_ERROR_…` constant macros record the source
`__FILE__` and `__LINE__` where they are used, in the `CHIP_ERROR` object.

Unless `CHIP_CONFIG_SHORT_ERROR_STR` is configured, the source file and
line appear in the result of `ErrorStr()`.

Fixes project-chip#8340 _Optionally tag CHIP_ERROR with source location_

#### Testing

- revised `src/lib/support/tests/TestErrorStr.cpp`
mspang pushed a commit that referenced this issue Jul 21, 2021
* Optionally tag CHIP_ERROR with source location

#### Problem

It can be difficult to identify the specific code that returned an error.

#### Change overview

Added a configuration option, `CHIP_CONFIG_ERROR_SOURCE`. When this is
enabled (and assuming `CHIP_ERROR` is a class, which should eventually
be true for all builds), `…_ERROR_…` constant macros record the source
`__FILE__` and `__LINE__` where they are used, in the `CHIP_ERROR` object.

Unless `CHIP_CONFIG_SHORT_ERROR_STR` is configured, the source file and
line appear in the result of `ErrorStr()`.

Fixes #8340 _Optionally tag CHIP_ERROR with source location_

#### Testing

- revised `src/lib/support/tests/TestErrorStr.cpp`

* Convert python interface

* review

* review

* Modify ReturnErrorOnFailure et al to accept CHIP_ERROR class or integer

* Fix flaky unit test

* restyle
mspang pushed a commit that referenced this issue Jul 21, 2021
* Make CHIP_ERROR a class type on ESP32.

#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue #8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on ESP32.

#### Testing

Passes esp32_qemu_tests.sh

* review
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 26, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on nrfconnect.

#### Testing

Existing tests should confirm no change to functionality.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 28, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on nrfconnect.

#### Testing

Existing tests should confirm no change to functionality.
mspang pushed a commit that referenced this issue Jul 28, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue #8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on nrfconnect.

#### Testing

Existing tests should confirm no change to functionality.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 29, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on k32w, mbed, and telink.

#### Testing

Existing tests should confirm no change to functionality.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 30, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on k32w, mbed, and telink.

#### Testing

Existing tests should confirm no change to functionality.
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Jul 30, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on k32w, mbed, and telink.

#### Testing

Existing tests should confirm no change to functionality.
andy31415 pushed a commit that referenced this issue Aug 3, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue #8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on k32w, mbed, and telink.

#### Testing

Existing tests should confirm no change to functionality.
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this issue Sep 23, 2021
* Make CHIP_ERROR a class type on Linux

#### Problem

Having `CHIP_ERROR` be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Added a transitional configuration option to select whether
  `CHIP_ERROR` is an integer (as now) or a class type. Initially, only
  the Linux platform is configured with a class. The intent is to
  incrementally fix integer assumptions in other platforms, and finally
  remove the configuration flag and streamline the `ChipError` class.

- Since logging calls had to modified anyway, added `FormatError()`,
  which can be configured to either return an integer value or a string
  (via `ErrorStr()`).

- Fixed some actual type errors, where non-CHIP_ERROR values were
  treated as CHIP_ERRORs. (None appear to have had more serious
  consequences than a nonsense message from ErrorStr().)

#### Testing

- Modified affected unit tests.
- Sanity check using chip-tool.

* review

* review

* zap regen
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this issue Sep 23, 2021
…p#8446)

#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on cc13x2_26x2, EFR32, QPG.

- Remove `operator bool()` (requested in project-chip#8383 review).

#### Testing

Existing tests should confirm no change to functionality.
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this issue Sep 23, 2021
* Optionally tag CHIP_ERROR with source location

#### Problem

It can be difficult to identify the specific code that returned an error.

#### Change overview

Added a configuration option, `CHIP_CONFIG_ERROR_SOURCE`. When this is
enabled (and assuming `CHIP_ERROR` is a class, which should eventually
be true for all builds), `…_ERROR_…` constant macros record the source
`__FILE__` and `__LINE__` where they are used, in the `CHIP_ERROR` object.

Unless `CHIP_CONFIG_SHORT_ERROR_STR` is configured, the source file and
line appear in the result of `ErrorStr()`.

Fixes project-chip#8340 _Optionally tag CHIP_ERROR with source location_

#### Testing

- revised `src/lib/support/tests/TestErrorStr.cpp`

* Convert python interface

* review

* review

* Modify ReturnErrorOnFailure et al to accept CHIP_ERROR class or integer

* Fix flaky unit test

* restyle
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this issue Sep 23, 2021
* Make CHIP_ERROR a class type on ESP32.

#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on ESP32.

#### Testing

Passes esp32_qemu_tests.sh

* review
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this issue Sep 23, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on nrfconnect.

#### Testing

Existing tests should confirm no change to functionality.
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this issue Sep 23, 2021
#### Problem

Having CHIP_ERROR be a class type would provide (a) type safety,
and (b) the ability to trace the source of errors (issue project-chip#8340).

#### Change overview

- Enable `CHIP_CONFIG_ERROR_CLASS` on k32w, mbed, and telink.

#### Testing

Existing tests should confirm no change to functionality.
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 a pull request may close this issue.

1 participant