Expose I2C ACK/NACK to the HLAs

Extensions
1

While working on the sigrok to Logic2 analyzer bridge I miss an important feature from the Logic2 exposed API: the ACK/NAK information is not present to the HLA analyzers, however in many times it would be useful.

2 Likes
2

That should be included.

Here you can see where they are added in the C++ I2C analyzer:

Here you can see one of our example HLAs reading it:

1 Like
3

We are missing the chip select enable/disable events in the SPI analyzer though, and plan to add that in the future.

1 Like
4

I might misunderstood something, but I was thinking about the bus state after the data/address bytes:

kép
kép1190×601 34.3 KB

But hey I just realized that the existing C++ analyzers can expose data to the HLAs? Well this will open up some possibilities.

5

Yup! but keep in mind, the API is unstable and subject to change as we iron out the kinks. Same is true for HLAs of course, something we need to improve our messaging on.

I might misunderstood something, but I was thinking about the bus state after the data/address bytes:

Oh right, sorry I didn’t think that through. You’re right, the ACK/NACK is missing from the frames. I’ve reported this as a bug, but if you would like to give it a try, you can open a PR on the I2C analyzer to add it! (Note, FrameV2 support is on the ‘alpha’ branch if the i2c-analyzer repository.)

On a side note, we setup cloud CI for all of our C++ analyzers over the weekend, with the hopes that folks who fork them could easily use the same CI provider to cross-build their analyzers.

2 Likes
6

Thanks for the answer Mark. You must be reading my mind, because my next question would be asking if you are open for PRs in this area :smiley:

Adding CI for the analyzers is a great idea! BTW. if we are talking about CI I would mention that it might be also useful to add a github action to run clang-formatter on each commit. Just to lower the stylistics issues during the reviews. If you happen to follow some existing coding style (which has existing clang-formatter descriptor) then I can help adding it.

7

Now you’re reading my mind!

I actually started adding clang-format to CI, but decided to skip for now and get back to it later.
This is the file we’re using for the main C++ codebase. (I added the txt extension to get our forum to accept it)
.clang-format.txt (967 Bytes)
If you open a PR to add this to any one of our analyzers, we have a templating tool which will allow us to easily roll out the changes to all of our analyzer repositories.

If you give it a try, please let me know if you have any pain points enabling CI for your fork! I want to write a guide for forking our analyzers and setting up CI eventually.

We’re using Azure Pipelines at the moment, I would like to just add clang-format to that. We could have used github actions instead, but I wanted to stick with azure pipelines because we’re using it for all of our internal repositories.

To enable clang-format, this block of code needs to be updated to call clang-format and inserted into the azure-pipelines yml file:

  - job: clangFormat
    displayName: 'clang format'
    pool:
      vmImage: 'ubuntu-16.04'
    steps:
    - script: |
        echo "TODO"

clang-format is already installed on the builder.

1 Like