Skip to content

EHL-2 i2c library#13

Open
sajidanower23 wants to merge 29 commits intomasterfrom
EHL-2-I2C-Library
Open

EHL-2 i2c library#13
sajidanower23 wants to merge 29 commits intomasterfrom
EHL-2-I2C-Library

Conversation

@sajidanower23
Copy link
Copy Markdown
Contributor

Science module lib

Copy link
Copy Markdown
Member

@hjed hjed left a comment

Choose a reason for hiding this comment

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

Lots of style comments. General logic looks good, but hard to tell without testing.

Lots of overlap between this and #12, I haven't repeated comments from #12

gnome-terminal --tab --command="bash -c '$openocd; $SHELL'" \
--tab --command="bash -c '$serial; $SHELL'" \
--tab --command="bash -c '$debug; $SHELL'" \
--disable-factory
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

would be better to use something more portable like tmux

@@ -0,0 +1,12 @@
Initialisation:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for all files that are in common please address comments on #12 before merging this (all the adc stuff)

@@ -0,0 +1,144 @@
#include "i2c.h"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

header comment

SysCtlPeripheralReset(sysctl_module_i2c[module]);
}

//GPIOIntRegister(i2c_module[module], i2cIntHandler) // might do interrupts later
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

commented code

}

/*
void i2c_write(i2cModule_t module, uint8_t msg[], size_t length) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

commented code

PUBLIC
${science-mod_headers}
)
endfunction()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is not the right place for this. Its not related to ros-echronos, probably belongs in the science-mod or /build-tools folder

include(${BUILD_TOOLS_DIR}/module_template.cmake)

#set(CPP_FILES adc_test.cpp can_wait_task.cpp)
set(CPP_FILES adc_test.cpp)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please include can_wait_task


/* Should never reach here, but if we do, an infinite loop is
easier to debug than returning somewhere random. */
for (;;) ;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

see comments on #12

i2c_select(I2C0, module_num); // select multiplexer output
// UARTprintf("Initialising science servo to neutral position\n");
// servo_init(SCIENCE_SERVO, SCIENCE_SERVO_PIN);
// servo_write(SCIENCE_SERVO, SCIENCE_SERVO_PIN, NEUTRAL_POS);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

commented code

// Create the publisher
ros_echronos::ROS_INFO("Data pub init\n");
owr_messages::science science_buffer_out[5];
ros_echronos::Publisher<owr_messages::science> science_pub("science/data", science_buffer_out, 5, false);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

these should all have topics starting with / I'd actually try and match these with the voltmeter one in terms of naming as well

@hjed
Copy link
Copy Markdown
Member

hjed commented Sep 13, 2018

Also this repo is BSB + AGPL license

@hjed
Copy link
Copy Markdown
Member

hjed commented Sep 21, 2019

@burrrrrr what's happening with this pr?

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