-
Notifications
You must be signed in to change notification settings - Fork 2
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
refactor: 🥅 Change all error handling to use errno, add int32_t return codes #25
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functions should return 0 if they succeeded, not if they failed. This is standard practice, which you'll see in use in projects such as the linux kernel, and LemLib/hardware
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
int
should be used instead ofuint32
.
PROS, and the other lemlib projects, return the maximum value of a 32-bit signed integer on failure, so this should do the same. -
possible errno values and their meanings should be documented in the function doxygen documentation. See LemLib/hardware documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the function return values and it should be good to merge
include/gamepad/button.hpp
Outdated
@@ -84,7 +84,7 @@ class Button { | |||
* gamepad::master.Up.onPress("upPress1", []() { std::cout << "I was pressed!" << std::endl; }); | |||
* @endcode | |||
*/ | |||
bool onPress(std::string listenerName, std::function<void(void)> func) const; | |||
uint32_t onPress(std::string listenerName, std::function<void(void)> func) const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the functions return INT_MAX
when errno is set, which is the maximum value of an int32_t
. Consider changing them to int32_t
for clarity, but this is not a requirement
uint32_t onPress(std::string listenerName, std::function<void(void)> func) const; | |
int32_t onPress(std::string listenerName, std::function<void(void)> func) const; |
include/gamepad/event_handler.hpp
Outdated
std::lock_guard lock(m_mutex); | ||
if (std::find(m_keys.begin(), m_keys.end(), key) != m_keys.end()) return false; | ||
if (std::find(m_keys.begin(), m_keys.end(), key) != m_keys.end()) return UINT32_MAX; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functions should return INT32_MAX
when errno is set, so it does not differ from PROS and other LemLib projects
if (std::find(m_keys.begin(), m_keys.end(), key) != m_keys.end()) return UINT32_MAX; | |
if (std::find(m_keys.begin(), m_keys.end(), key) != m_keys.end()) return INT32_MAX; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, although i'd like for there to be more example code in the documentation
Download the template for this pull request:
Note
This is auto generated from
Add Template to Pull Request