SRA-VJTI / sra-board-component

ESP-IDF component for SRA Board
https://sravjti.in/sra-board-component/
MIT License
36 stars 20 forks source link

Redundant error variables #17

Closed laukik-hase closed 3 years ago

laukik-hase commented 3 years ago

https://github.com/SRA-VJTI/sra-board-component/blob/d58505f71440e80b5fe426d02a260f7a3d6b5264/src/adc.c#L8

@ombhilare999 Use a single error variable esp_err_t errfor all the purposes. This has already been discussed with @VedantParanjape

VedantParanjape commented 3 years ago

@ombhilare999 Make changes only after syncing your fork with the upstream repository.

VedantParanjape commented 3 years ago

@ombhilare999 has this issue been fixed ?

laukik-hase commented 3 years ago

We can do something like this -

//Macro for error checking
#define IS_ESP_OK(x, label) \
  if ((x) != ESP_OK)        \
    goto label;

esp_err_t foo()
{
    esp_err_t err = ESP_FAIL;

    IS_ESP_OK(func1(), EXIT);
    IS_ESP_OK(func2(), EXIT);
    IS_ESP_OK(funcN(), EXIT);

   err = ESP_OK;
EXIT:
    return err;
}
VedantParanjape commented 3 years ago

Rather than using goto, this is a better solution

#define CHECK(x) do { esp_err_t __; if ((__ = x) != ESP_OK) return __; } while (0)

Used in almost all drivers found here: https://github.com/UncleRus/esp-idf-lib

VedantParanjape commented 3 years ago

@laukik-hase Review the commit 1a23093117c9ea1e9f876168f3365ca6997dde12 and please close the issue if it looks alright

laukik-hase commented 3 years ago

https://github.com/SRA-VJTI/sra-board-component/blob/1a23093117c9ea1e9f876168f3365ca6997dde12/src/servo.c#L3

@VedantParanjape The error-checking macro is included in every file, can we define it in the helper functions file, so that every component will have direct access to it?

VedantParanjape commented 3 years ago

https://github.com/SRA-VJTI/sra-board-component/blob/1a23093117c9ea1e9f876168f3365ca6997dde12/src/servo.c#L3

@VedantParanjape The error-checking macro is included in every file, can we define it in the helper functions file, so that every component will have direct access to it?

I wanted the macro to be private, i.e. it shouldn't be accessed by end user, so I preferred putting macro in each source file.

laukik-hase commented 3 years ago

https://github.com/SRA-VJTI/sra-board-component/blob/1a23093117c9ea1e9f876168f3365ca6997dde12/src/servo.c#L3

@VedantParanjape The error-checking macro is included in every file, can we define it in the helper functions file, so that every component will have direct access to it?

I wanted the macro to be private, i.e. it shouldn't be accessed by end user, so I preferred putting macro in each source file.

But, if new components are to be added, the user will have to take care of that. Also, it is just a error checking macro, the user cannot do much wrong with it.

VedantParanjape commented 3 years ago

https://github.com/SRA-VJTI/sra-board-component/blob/1a23093117c9ea1e9f876168f3365ca6997dde12/src/servo.c#L3

@VedantParanjape The error-checking macro is included in every file, can we define it in the helper functions file, so that every component will have direct access to it?

I wanted the macro to be private, i.e. it shouldn't be accessed by end user, so I preferred putting macro in each source file.

But, if new components are to be added, the user will have to take care of that. Also, it is just a error checking macro, the user cannot do much wrong with it.

Ohk, I'll do it