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

aivision for beta release #729

Open
wants to merge 10 commits into
base: develop-pros-4
Choose a base branch
from

Conversation

Rocky14683
Copy link
Member

Summary:

ai vision sensor for beta release(so no docs for cpp)

WillXuCodes and others added 7 commits October 25, 2024 22:34
🐛 use `pros` instead of `prosv5` in Makefile
* move version information

* update scripts and makefiles

* add version.h to main.h

* fix bugs on windows and linux

* move version.h to pros folder

* update header comments
@Rocky14683 Rocky14683 force-pushed the pros-4/feature/aivision branch from 47d2af4 to 7d24160 Compare December 10, 2024 02:01
Copy link
Member

@WillXuCodes WillXuCodes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor fixes, not commenting on functionality for now because my impression is that this hasn't been hardware tested yet.

@@ -40,6 +40,7 @@
#endif /* __cplusplus */

#include "pros/adi.h"
#include "pros/aivision.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm not really a big fan of the "aivision" name, at least make it ai_vision or ai_camera (underscore matters the most, vision might be a little to ambigious)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(actually vision is fine but I'd prefer a underscore between ai and vision just for readability's sake)

namespace pros {
inline namespace v5 {

enum class AivisionDetectType : uint8_t { color = (1 << 0), code = (1 << 1), object = (1 << 2), tag = (1 << 3) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be nice to have this on seperate lines for readability's sake:

Suggested change
enum class AivisionDetectType : uint8_t { color = (1 << 0), code = (1 << 1), object = (1 << 2), tag = (1 << 3) };
enum class AivisionDetectType : uint8_t {
color = (1 << 0),
code = (1 << 1),
object = (1 << 2),
tag = (1 << 3)
};

Comment on lines +31 to +33
claim_port_i(port - 1, E_DEVICE_AIVISION);
uint8_t enabled_detection_types = vexDeviceAiVisionEnableGet(device->device_info);
return_port(port - 1, enabled_detection_types);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
claim_port_i(port - 1, E_DEVICE_AIVISION);
uint8_t enabled_detection_types = vexDeviceAiVisionEnableGet(device->device_info);
return_port(port - 1, enabled_detection_types);
claim_port_i(port - 1, E_DEVICE_AIVISION);
uint8_t enabled_detection_types = vexDeviceAiVisionEnableGet(device->device_info);
return_port(port - 1, enabled_detection_types);

Inconsistent spacing


int32_t aivision_get_class_name(uint8_t port, int32_t id, uint8_t * class_name) {
claim_port_i(port - 1, E_DEVICE_AIVISION);
vexDeviceAiVisionClassNameGet(device->device_info, id, class_name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here, it's a spaces vs tabs issue (we have a formatter on kernel right?)

return c::aivision_set_code(this->_port, &code);
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline

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