Kb/feature/add tactile sensing and haptic feedback#7
Conversation
Juliaj
left a comment
There was a problem hiding this comment.
Overall the code looks good to me. This is a good addition to the original upstream repo, let's add it to FORK.md. In terms of location, how do you feel about moving Sensors to under src ? Moving it there make it easier to release this as standalone Python package for other applications to reuse.
There are some Python styling issues which our pre-commit wasn't able to catch. We also missed this with previous check-in. I am planning to fix them after your PR, changes are parked at branch jj/refactor/enhance_ruff_check.
| |---|---| | ||
| | Camera | https://www.amazon.com/dp/B0CLRJZG8D | | ||
| | AD/DA Expansion Hat | https://www.amazon.com/dp/B09Q8Y5F7Y | | ||
| | Haptic Motors | https://www.amazon.com/dp/B0FX9HW33Z | |
There was a problem hiding this comment.
This motor is out of stock. Is this a good replacement https://www.amazon.com/Vibration-9000RPM-Replacement-Arduino-Projects/dp/B08GPMCP7J/140-3617096-9051911?psc=1 ?
There was a problem hiding this comment.
updated with commit. Let me know what you think.
For the src move:
Keeping sensors as a sibling package (amazinghand_sensors/) rather than a subpackage lets users install the core SDK without hardware-specific deps like lgpio and spidev. Each package gets its own pyproject.toml, versions independently, and declares only the deps it actually needs.
Is this ok or do I need to account for something else I may have missed?
|
Also, have you been able to run this code successfully on your setup with all the hardware -- any challenges or questions on setting up your system? |
| raise ValueError(f"ADS1256 single-ended channel must be 0–7, got {channel}") | ||
| self._write_reg(REG["REG_MUX"], (channel << 4) | 0x08) | ||
|
|
||
| def _set_diff_channel(self, channel: int): |
There was a problem hiding this comment.
_set_channel and _set_diff_channel both use channel as input argument. It seems that the pattern is standard even though the semantic for channel is different, physical channel versus channel pair. A small clarify improvement is renaming the channel here to pair_index, diff_channel or something similar.
There was a problem hiding this comment.
Renamed to diff_channel and added a comment clarifying it's a pair index (0–3), not a physical AIN pin number, with the pin mapping spelled out.
| logger.debug("Ramping up") | ||
| for duty in range(0, 101, step): | ||
| self.motor.value = duty / 100.0 | ||
| sleep(delay_s) |
There was a problem hiding this comment.
The sleep(...) here is fine. If we tighten the timing precision, we could use a monotonic clock. For example,
try:
next_tick = monotonic()
while True:
logger.debug("Ramping up")
for duty in range(0, 101, step):
self.motor.value = duty / 100.0
next_tick += delay_s
sleep(max(0.0, next_tick - monotonic()))
logger.debug("Ramping down")
for duty in range(100, -1, -step):
self.motor.value = duty / 100.0
next_tick += delay_s
sleep(max(0.0, next_tick - monotonic()))
next_tick += pause_s
sleep(max(0.0, next_tick - monotonic()))
...
There was a problem hiding this comment.
added. and left a comment for time.perf_counter for future if we need sub 1 ms accuracy
… cleanup issues, remove dead code and unused imports, and register the package as an editable pixi dependency
|
@Juliaj can you review again when you get a chance? thanks |
|
@kbhakt, I tested out the tactile sensor and haptic coin. The python scripts ran without any issue. Haptic coin behaves as expected. For tactitle sensor, I used 10 kΩ resister (config.toml was updated with fsr_r_fixed = 10000). The sensor was wired to AIN3 (thumb). I noticed following
|
When I press the sensor with the 5.1 kOhm resistor I get my voltage to go to around ~2.3V (note the magnitude and sensitivity of this will depend on your resistor value you choose). For your floating wires, hard to know what the noise will be but should not worry about it if you're not using those channels. One thing I have noticed is that if you use AIN1 sometimes that can be a noisier signal than using AIN7. not sure if this is a shielding issue but something I have observed in the past. |



Adds initial support for FSR-based tactile sensing and haptic coin motor feedback.
Changes:
Initial scripts for haptic coin motor and tactile FSR sensor
Haptic test script
Resolved PySide6 dependency
Bug fixes in tactile sensing
Updated README with setup instructions and BOM
Added sensor dependencies to pixi.lock and updated CI