'scope firmware DMA settings edits

[brentfpage]

These edits to the DMA settings in the firmware might improve the performance of #367 at frequencies above 25 khz, but it's not obvious that they do.
Changes like those to line 17,
DMA.CH0.ADDRCTRL = DMA_CH_SRCRELOAD_BURST_gc | DMA_CH_SRCDIR_INC_gc | DMA_CH_DESTDIR_INC_gc | DMA_CH_DESTRELOAD_BLOCK_gc; //Source reloads after each burst, with byte incrementing. Dest does not reload, but does increment address.
->
DMA.CH0.ADDRCTRL = DMA_CH_SRCDIR_FIXED_gc | DMA_CH_DESTDIR_INC_gc | DMA_CH_DESTRELOAD_BLOCK_gc; //Source reloads after each burst, with byte incrementing. Dest does not reload, but does increment address.
may just be a matter of taste, but it seems possible that the second method takes fewer steps. From what I can tell, these transfers are both 1 byte per burst and 1 byte per transfer, so 1 transfer per burst. If DMA_CH_SRCRELOAD_BURST_gc | DMA_CH_SRCDIR_INC_gc is set, the source address gets incremented after a transfer, but because that is equal to a burst, it then gets immediately set to the original address. This might be achieved more directly using DMA_CH_SRCDIR_FIXED_gc.

I'm more confident in the change of the DMA CH1 trigger source to DMA_CH_TRIGSRC_ADCA_CH2_gc in tiny_dma_set_mode_2. In the current firmware, it's instead set to ...ADCA_CH0_gc, but the source address for DMA CH1 is ADCA_CH2. It appears this discrepancy almost never or never matters because both ADC_CH0 and ADC_CH2 are sampling at the same frequency in this mode, but maybe it rears its head in some scenarios.

[mi-hol]

The questions in #385 (comment) are linked with merging this PR

[mi-hol]

The comment from #260 (comment)
would seem to apply for this PR too.

My firmware was overwritten by the default firmware and increasing the FIRMWARE_VERSION_ID solves the issue.

[mi-hol]

@brentfpage did you see above comment #381 (comment)?
I miss an update for FIRMWARE_VERSION_ID in the changed files :)

[brentfpage]

@mi-hol Good point, done now

[mi-hol]

The log shows no files named labrafirm_0008* as I would have expected.

1. Is my expectation maybe wrong
   OR
2. Are we missing a "build firmware" action or other/additional changes?

[brentfpage]

I don't think there's an action for building the firmware.

[mi-hol]

action for building the firmware.

@brentfpage creating this missing workflow, would that be a challenge you'd like to own?

[brentfpage]

action for building the firmware.

@brentfpage creating this missing workflow, would that be a challenge you'd like to own?

Started a discussion in #396

[mi-hol]

Started a discussion in #396

@brentfpage from your comment there I assume a new PR for an action that builds and uploads 2 firmware file to Labrador firwmare directory.
Is that correct understanding?
This new PR would then unblock merging this PR

[brentfpage]

Started a discussion in #396

@brentfpage from your comment there I assume a new PR for an action that builds and uploads 2 firmware file to Labrador firwmare directory. Is that correct understanding? This new PR would then unblock merging this PR

That sounds good. I'm not sure what the best way to do it is though. Is there a way to modify the Labrador repo using a build action– specifically, to upload firmware files to this directory in this case? From what I've gathered, the "actions/checkout@v4" line at the top of all the workflow files actually clones the repo, so there's no easy access to the genuine repo.

Also, it would be nice to have some sort of automatic versioning so that FIRMWARE_VERSION_ID in AVR_Code/..../globals.h and EXPECTED_FIRMWARE_VERSION in Desktop_Interface/genericusbdriver.h get incremented when the avr code is updated. I hope to add that functionality.

[mi-hol]

I'm not sure what the best way to do it is though. Is there a way to modify the Labrador repo using a build action– specifically, to upload firmware files to this directory in this case?

updated:
Uploading firmware files to this directory is the correct solution.
An example I know is on https://github.com/FOME-Tech/fome-fw/blob/master/.github/workflows/build-firmware.yaml
Does that help?

[mi-hol]

it would be nice to have some sort of automatic (firmware) versioning

updated:
Considering the currently existing pain points of firmware versioning, automating versioning
would have lower priority from my view and I'd recommend to address it only after the first part (above) was achieved.
Main reason is that a better solution than currently used "file naming approach" is required from my view.

[brentfpage]

Uploading firmware files to this directory is the correct solution.
An example I know is on https://github.com/FOME-Tech/fome-fw/blob/master/.github/workflows/build-firmware.yaml

Maybe I'm missing something, but doesn't

    - name: Upload ini
      uses: actions/upload-artifact@v4
      with:
        name: ${{matrix.ini-file}}
        path: ./firmware/tunerstudio/generated/${{matrix.ini-file}}

in that workflow upload from ./firmware/tunerstudio/generated/, not to it? I see that /firmware/tunerstudio/generated/ in the genuine repo does actually contain firmware files, but I'm guessing they're added to it separately for the following reasons:

• In Labrador mac.yml,

      - name: Upload artifacts
        if: ( env.MACOS_CERTIFICATE != '' && env.MACOS_CERTIFICATE_PWD != '' )  
        uses: actions/upload-artifact@v4
        with:
          name: asset-dmg
          path: Desktop_Interface/Labrador*.dmg

uploads Desktop_Interface/Labrador*.dmg from the workflow's clone of the repo. The file Desktop_Interface/Labrador*.dmg never appears in the genuine repo.

• this suggests that pushing changes from the workflow clone to the genuine repo requires doing something like configuring personal access tokens (PAT) and adding references to them in the workflow file

To clarify, I think the following happens:

• when build-firmware.yaml is run:
  • firmware files are installed in ./firmware/tunerstudio/generated/ in the workflow's clone of the repo
  • upload_artifacts uploads these to a directory accessible to the developers, similarly to how upload_artifacts in mac.yml results in Labrador.dmg being downloadable via a link provided by github
  • the workflow finishes, the clone of the repo is deleted, and ./firmware/tunerstudio/generated/ in the genuine repo has not been touched
• some other mechanism later uploads the artifacts into ./firmware/tunerstudio/generated/ in the genuine repo

[mi-hol]

uploads Desktop_Interface/Labrador*.dmg from the workflow's clone of the repo. The file Desktop_Interface/Labrador*.dmg never appears in the genuine repo.

• this suggests that pushing changes from the workflow clone to the genuine repo requires doing something like configuring personal access tokens (PAT) and adding references to them in the workflow file

Updated:
Maybe we should break with manual legacy process for firmware and stop uploading compiled firmware files to the repo.
That change would solve the issue of obsolete, orphaned firmware files in repo too!

Maybe the better place for handling the new process would be in https://github.com/espotek-org/Labrador/blob/master/.github/workflows/continuous.yml ?
@turboencabulator maybe, as the author of continuous.yml, you'd have a recommendation ?

[turboencabulator]

I don't currently have any recommendations, but I have lots of thoughts and opinions!

Putting generated files in a repo: Almost always a bad idea. In my experience some generated files aren't binary reproducible, they've got embedded timestamps or paths, or the output depends on the exact version of your compiler or OS. Alice commits a generated file, Bob clones the repo and builds it, and now Bob sees the generated file in a modified state. Maybe Bob ignores it, or maybe he commits it and now Alice has that same problem. These are almost always large binary files that bloat the repo. My .git directory for this project is currently about 140MB because even after you delete a file, git still has to remember it.

Automated commits (either to commit firmware files or to bump all the version numbers): Probably more trouble than it's worth, and there are more ways for it to go wrong than to go right. I know there is a lot of trial and error to get the automation working correctly, while firmware isn't going to see many updates to make the cost vs. benefit worth it. I'd suggest documenting the instructions somewhere and using that as a checklist for how to release the firmware in the future.

Just like we don't need to run the release process on every commit, I doubt we need to bump the firmware version numbers on every commit that touches firmware. My preference is to increment it when there is a new feature or improvement, and after we think it's stable (basically the same reasons to run the continuous workflow). The problem is that the desktop software forces you to use a particular firmware version, making proposed firmware changes hard to test.

We're getting pretty off-topic for a PR originally about DMA settings. If those DMA changes work, then let's do it and move this discussion elsewhere for what we want the process to look like.

[mi-hol]

I doubt we need to bump the firmware version numbers on every commit that touches firmware.

@turboencabulator According to Chris's comment #260 (comment), firmware gets only applied to hardware when the firmware version number is incremented. So this is a fact!

If those DMA changes work, then let's do it and move this discussion elsewhere for what we want the process to look like.

I'd be happy to do as you suggest, unfortunately to test for Windows, the installer config (Desktop_Interface/build_win/Labrador.aip) will need to be modified. Not sure about about the LINUX installers because searching for "firmware" and "labrafirm" had no hits in the related "build_*" dirs.
Could you comment please?

[mi-hol]

Note: related PR #402 should be merged first

[turboencabulator]

@turboencabulator According to Chris's comment #260 (comment), firmware gets only applied to hardware when the firmware version number is incremented. So this is a fact!

Firmware gets automatically loaded whenever the version currently on the hardware doesn't match whatever the desktop software expects.

Labrador/Desktop_Interface/genericusbdriver.cpp

Lines 484 to 491 in c9dd7f2

	if((firmver != EXPECTED_FIRMWARE_VERSION) || (variant != DEFINED_EXPECTED_VARIANT)){
	qDebug() << "Unexpected Firmware!!";
	int flashRet = flashFirmware();
	qDebug("flashRet: %d", flashRet);
	connected = false;
	connectTimer->start();
	return;
	}

Or you can fire up dfu-programmer and load what you want, but you have to watch out for this automatic loading to overwrite it.

The point I was trying to make earlier is that this change doesn't break compatibility with the desktop software (at least I don't think it does). If users won't see any difference, do we really need to make a new firmware version right now and force the desktop software to load that version?

[turboencabulator]

I'd be happy to do as you suggest, unfortunately to test for Windows, the installer config (Desktop_Interface/build_win/Labrador.aip) will need to be modified. Not sure about about the LINUX installers because searching for "firmware" and "labrafirm" had no hits in the related "build_*" dirs. Could you comment please?

Check Desktop_Interface/Labrador.pro. Usually all the firmware files in the directory get copied into the package (no need to list them individually), except Android which only copies a single firmware file to make the package size smaller.

[mi-hol]

that this change doesn't break compatibility

Sorry, I fail to understand this reasoning.
A change was made in firmware and needs to be tested, due to automatic firmware update, making a new firmware version, seems the only approach to reliably deploy it.
It's all about simplicity and avoiding unexpected side effects.