intelligent-agent / redeem

Firmware for Replicape
http://wiki.thing-printer.com/index.php?title=Redeem
GNU General Public License v3.0
36 stars 44 forks source link

Address #181 : Fail to set A4A motor current on 2.2.1-RC2 #182

Closed zaped212 closed 5 years ago

zaped212 commented 5 years ago

Umikaze 2.2.1-RC2 includes updated versions of Adafruit-BBIO This new library changes how the SPI module works.

This changes the DAC class to match the updated SPI module to use /dev/spidev2_0

zaped212 commented 5 years ago

Hey guys, I dont think this is necessarily the final solution but it is working for me now. Confirmed motors now work, and verified Avref and Bvref were set correctly using a multimeter.

zaped212 commented 5 years ago

What are your guy's thoughts about making Adafruit-BBIO version 1.1.1 a requirement? I can test on my A4A board on 2.2.1-RC2, along with 2.1.1 to make sure things work.

Looks like we could put the version in setup.py.

ThatWileyGuy commented 5 years ago

I don't have any strong opinions about Adafruit-BBIO versions.

I do think the long-term answer here is to stop opening the SPI busses by numbers - the numbers aren't guaranteed even across reboots because the kernel will probe the SPI masters in an undefined order. That leaves two approaches I can think of...

The first would be to probe /sys based on the SPI master we know we're supposed to use and open it based on that. On my 00B3 board, I see that /sys/devices/platform/ocp/481a0000.spi/spi_master/spi2/spi2.1/ exists, which I believe implies that /dev/spidev2.1 will correspond to the SPI master at 481a0000 in the BBB's physical memory space. What does this look like on your A4A?

The second would be to try to find the hardware we're looking for in /dev/spidev* and rely on SPI bus failures to tell us when we have the wrong bus.

Any thoughts?

In the short term, I don't have any problem merging this in as long as we know we aren't breaking any other boards in the process. Is A4A the only board using this DAC class?

zaped212 commented 5 years ago

On: 2.2.1-RC root@kamikaze:/sys/devices/platform/ocp/481a0000.spi/spi_master/spi2# pwd /sys/devices/platform/ocp/481a0000.spi/spi_master/spi2 root@kamikaze:/sys/devices/platform/ocp/481a0000.spi/spi_master/spi2# ls device of_node power spi2.0 spi2.1 statistics subsystem uevent

So that seems to match up with what you are thinking.

From Stepper.py: Stepper_00A4 Stepper_reach_00A4 ( inherits Stepper_00A4 ) Stepper_00A3 ( inherits Stepper_00A4 ) use the DAC.

B1, B2, B3 all use PWM_DAC instead.

zaped212 commented 5 years ago

@ThatWileyGuy I dont think this change should be merged exactly as it is. as it will break any a4a boards running umikaze 2.1.1or older because they have the older version of the BBIO library.

I think if we want to merge this in, then we would need to enforce the BBIO version.

Wackerbarth commented 5 years ago

In order to use this version of Redeem, why shouldn't we require that everyone upgrade their kernel and OS installation? When we created 2.1.x, the intention was that it was for a 4.04 kernel and 2.2.x was intended for the newer one. At some point, it doesn't make sense to attempt to support legacy installations.

Wackerbarth commented 5 years ago

@ThatWileyGuy -- Although I acknowledge that you don't like the idea of putting such parameters in a configuration file that the user might be able to change, doing so would provide a mechanism to work around issues such as this.

goeland86 commented 5 years ago

@Wackerbarth is right - we did decide that the 2.2.x+ versions of Redeem would only work on kernels 4.14 and newer. The 2.1.x branch is no longer actively developed. I would suggest that we mark it as deprecated, tag it and remove it from the actively developed branches.

@zaped212 anyone merging the current Redeem code onto a 2.1.1 Umikaze image will find things breaking anyway, due to other changes that have been made. Thus I would argue that it's not an issue to push the code as @ThatWileyGuy proposed it. We should enforce a version of BBIO anyway to be on the safe side with newer images being generated, but there's no intent to be backwards compatible to infinity either.

zaped212 commented 5 years ago

I am good with adding the BBIO version enforcement. I think that would help address issues in the future by having a known version of BBIO.

Am I correct in saying that I would need to modify: platforms=["BeagleBone"], install_requires=[ ... 'Adafruit_BBIO==1.1.1',

Wackerbarth commented 5 years ago

The requirement Adafruit_BBIO==1.1.1 is too restrictive and therefore not appropriate. Adafruit_BBIO>=1.1.1 might be considered. If we later find that some newer version is incompatible, we can add additional constraints. The default specification should be kept as simple as possible as long as some other constraint (such as using a particular OS base) eliminates any earlier versions which are incompatible.

zaped212 commented 5 years ago

@Wackerbarth sounds good. I will use the >= 1.1.1 requirement. I will try and get a change pushed up later tonight.