BITalinoWorld / revolution-python-api

Python API for BITalino (r)evolution
GNU General Public License v3.0
21 stars 19 forks source link

State function not compatible with bitalino firmware 5.2 #13

Closed morganOmind closed 2 years ago

morganOmind commented 2 years ago

Hi, I have an issue with a new bitalino board running firmware 5.2. After investigating, it seems to be linked with the state function of the python api. In this firmware commit (https://github.com/BITalinoWorld/firmware-bitalino-revolution/commit/457cbfbcec6d5499964fa67d16cfbe23701021e0) the crc value of the sendStatus function has been modified, but it seems that the python api state function has not been updated accordingly, The test line if (crc == x & 0x0F): is still the same that before the firmware change, but now it raises the ExceptionCode.CONTACTING_DEVICE exception since the test returns False due to the firmware change. Could you please fix this in the python api?

morganOmind commented 2 years ago

I guess the crc test should take into account the firmware version of the board in order to work properly with the different versions..

morganOmind commented 2 years ago

Notice that in its current state, the state function of the python api works properly with boards running firmware 5.1.

fcachado commented 2 years ago

Dear @morganOmind,

I could not replicate your issue using a BITalino v5.2.

Can you describe the steps taken in order to replicate the issue?

morganOmind commented 2 years ago

Hi @fcachado Thanks for your reply. Trying to run this code with a bitalino board 5.2:

# Connect to BITalino
try:
    self.device = BITalino(port)
except UnicodeDecodeError:
    # This can happen after an internal buffer overflow.
    # The solution seems to power off the device and repair.
    raise WorkerInterrupt("Unstable state. Could not connect.")
except Exception as e:
    raise WorkerInterrupt(e)

# Set battery threshold
# The red led will light up at 5-10%
self.device.battery(30)

# Read BITalino version
self.logger.info(self.device.version())

# Read state and show battery level
# http://forum.bitalino.com/viewtopic.php?t=448
state = self.device.state()

it raises the exception when trying to execute the last line.

morganOmind commented 2 years ago

while when running the same code with a board 5.1, it works just fine..

fcachado commented 2 years ago

Thank you for your fast reply!

I could now replicate the issue and I already fixed it. I will make a commit in the next few minutes and push it to the PyPi.

morganOmind commented 2 years ago

Oh it's great, thank you very much for your reactivity and efficiency :)

fcachado commented 2 years ago

New version available at PyPI.

morganOmind commented 2 years ago

Hi, Yes, I just tested it and the fix works great thank you. I tested both v5.1 and v5.2 and everything seems ok. Except one thing (sorry!), now the baterry line of my previous code return Battery: 5245.46% only with the v5.2 board. I think its linked to the same commit change as the previous fix.. (and just in case you fix and release a new version, could you please also decrease the constraint on the numpy package version in the setup.py file? Unless it is strictly necessary for the bitalno package to work properly, but it seems not to be from the tests I ran. Because of others dependencies that are not up-to-date, could you for instance require numpy>=1.19 rather than pointing a specific version?)

morganOmind commented 2 years ago

sorry I forgot to put the code!

# Read state and show battery level
# http://forum.bitalino.com/viewtopic.php?t=448
state = self.device.state()
battery = round(1 + (state["battery"] - 511) * ((99 - 1) / (645 - 511)), 2)
self.logger.info("Battery: %.2f%%", battery)
morganOmind commented 2 years ago

Hi, sorry to bother you, but I'm a little bit in a rush with this issue since I should launch my new setup using the bitalino device in a week in pre-production, and this battery state issue is a bit limiting for my use case.. Do you think you will be able to fix it in a near future please? I tried to look for a simple fix, but honestly I am not an expert of such low-level microcontroller firmware development!

morganOmind commented 2 years ago

Hello again. Finally I think I have a fix :

`

                digitalPorts.append(decodedData[-1] >> 7 & 0x01)
                digitalPorts.append(decodedData[-1] >> 6 & 0x01)
                digitalPorts.append(decodedData[-1] >> 5 & 0x01)
                digitalPorts.append(decodedData[-1] >> 4 & 0x01)

                offset = 0;
                if self.isBitalino52:
                    offset = -1

                batteryThreshold = decodedData[-2 + offset]
                battery = decodedData[-3 + offset] << 8 | decodedData[-4 + offset]
                A6 = decodedData[-5 + offset] << 8 | decodedData[-6 + offset]
                A5 = decodedData[-7 + offset] << 8 | decodedData[-8 + offset]
                A4 = decodedData[-9 + offset] << 8 | decodedData[-10 + offset]
                A3 = decodedData[-11 + offset] << 8 | decodedData[-12 + offset]
                A2 = decodedData[-13 + offset] << 8 | decodedData[-14 + offset]
                A1 = decodedData[-15 + offset] << 8 | decodedData[-16 + offset]
                acquiredData = {}
                acquiredData["analogChannels"] = [A1, A2, A3, A4, A5, A6]
                acquiredData["battery"] = battery
                acquiredData["batteryThreshold"] = batteryThreshold
                acquiredData["digitalChannels"] = digitalPorts
                return acquiredData`

Pretty simple at the end!!

fcachado commented 2 years ago

Dear @morganOmind,

Using your suggestions, I released a new version here in GitHub, and also it is already available in PyPi (1.2.6).

I will await your comments, hoping that this new release will fix your issues.

morganOmind commented 2 years ago

Thanks a lot, it is perfectly working now. And also thanks for the requirements change (i.e numpy) that simplify my use case!