search

RIT-MDRC / Catbot

Main repository for catbot onboard software
MIT License
1 stars 0 forks source link

Potentiometer (C++) #24

Closed thatnoobles closed 3 months ago

thatnoobles commented 4 months ago

Library code to read potentiometer values.

Communicates with potentiometers through Catbot's 12-bit ADC. Can support multiple ADCs, though currently the code only references the one at I2C address 0x48.

To compile, run the make command in src/raspi/cpp, which will generate the file potentiometer.so (the Python module). There is a sample Python script, src/raspi/cpp/io_controller/potTest.py, that will read POT0.

(more information is viewable at the ADC's datasheet, relevant content starts at page 11)

thatnoobles commented 3 months ago

@hiromon0125

  1. I've made a commit that stores ADC handles/addresses as arrays, and each potentiometer is initialized with an ADC index that associates it with a specific item in each array. In theory we could add more ADCs to the robot by adding more entries to these arrays.
  2. Parallelism is limited by hardware. We only have one I2C bus, so we can only do one read/write operation on the bus at a time. However, I2C is fast enough where we can poll potentiometers in sequence and it will be basically simultaneous.
  3. To my knowledge--and from what I've seen--the I2C line is closed when the software is killed, since the process is no longer using it. If this ends up causing issues then we can add some simple cleanup code.
  4. Performance, mostly. Even though the C++ code will only be run as fast as the Python code can call it, this does narrow down any performance issues to our Python code. Plus being able to enforce variable sizes (e.g. being able to deal in bytes) is nice.
  5. Sure, makes sense :+1:
hiromon0125 commented 3 months ago
  1. I've made a commit that stores ADC handles/addresses as arrays, and each potentiometer is initialized with an ADC index that associates it with a specific item in each array. In theory, we could add more ADCs to the robot by adding more entries to these arrays.

Okay. Sounds good. I will take a look once you guys make it out of draft.

  1. Parallelism is limited by hardware. We only have one I2C bus, so we can only do one read/write operation on the bus at a time. However, I2C is fast enough that we can poll potentiometers in sequence and it will be basically simultaneous.

Yeah, I just want this class to be parallelism-safe such that anything we end up doing at the behavior level, we don't need to come back and change this. This idea has been incorporated in Python too therefore I want you guys to follow this as well.

  1. To my knowledge--and from what I've seen--the I2C line is closed when the software is killed, since the process is no longer using it. If this ends up causing issues then we can add some simple cleanup code.

I want this for good practice. Python architecture is adding this in the next PR for I2C, so I want you guys to do this as well.

  1. Performance, mostly. Even though the C++ code will only be run as fast as the Python code can call it, this does narrow down any performance issues to our Python code. Plus being able to enforce variable sizes (e.g. being able to deal in bytes) is nice.

I don't want to make the system complex from early on for "performance". This could bite us overall like how we already are falling behind schedule. I don't see how being able to deal with bytes in C code is different from Python. I just threw your code into ChatGPT and made it translate to Python and it threw out the same syntax. If "being able to deal in bytes" is just a bit shifting operation then this is not worth it because Python can do this as well. I also worry about memory manipulation and open any future developer to unsafe code practices. Reshaping the software architecture to prevent such a situation from early on will greatly improve our future developers, and we need to make sure this is one of our priorities.

  1. This might be more related toward the entire cpp source code organization as a whole, but the Python file structure is separated into components for import syntax purposes. I was wondering if cpp code could also follow a similar structure and have each cpp class live directly next to the python file that uses the class. It might be easier to find the cpp class definition from the Python script if it is organized that way. This will get rid of the Python and cpp directories, and makefile also does not have much of an issue either since you are pointing to each cpp file that needs to be compiled. Less difficulty navigating through differently organized folders might be worth investigating? Let me know your thoughts

Also any thoughts on this?

I am also mostly concerned about how much time this is taking you guys to complete. If hardware is the reason for the delay then software needs to adapt in such a way that we can develop independently from hardware; I told Danny to make this a priority. If the only way you guys can test the code is directly on the hardware, then you are also making the test susceptible to hardware issues, meaning you are testing the part you developed while also testing something you don't need to test. Also not to mention the time we are waiting for hardware fix should be utilized more with something else. I like you guys to make this PR out of the draft phase by the end of next Thursday's meeting. I would also like you guys to start work on Documentation for this, and what ideas have been applied to solve any problems we might face in the future. While I did agree to let you guys develop some C code, if there is no overarching benefit from developing in C that heavily outweigh the con of complex architecture, development time, and any future risk with new members, then we can not let this code into main branch. I will have a system for counting timer in Python soon and we can compare whether this C code is really more performant or it is just being a distraction. I also do not want any excuse that python program is slowing down your performance because ultimately we are weighing the portion of code you replaced and comparing the section you have developed with the rest of the system and seeing if they are worth investing our time. I also want development time as one of our main factors as well so if something that takes three developers to develop in a little less than half a semester can be achieved by one developer in 2 weeks then that is a huge down side.

I will also not going to approve a draft PR, so I need you guys to make this PR official before I make the next review.