DiSlord / NanoVNA-D

Firmware for NanoVNA, NanoVNA-H, NanoVNA-H4. Support SD Card, external Serial connection, fast measure, fast exchange vs CPU
272 stars 63 forks source link

Provide a command line option to build FW for the MS hardware variant. #54

Open Ho-Ro opened 1 year ago

Ho-Ro commented 1 year ago

Add build support for the MS HW variant as proposed in this discussion.

Bingo600 commented 1 year ago

Thanx Martin for opening a "ticket" (Carsten here).

I have a suggestion (patch against head) here , but i'm not sure about the naming convention used

patch-hw-variant.diff.txt

It's just a suggestion

If going full configurable from si5351.h , we could also DEFINE the SI/MS CHIP Strings in si5351.h

It defaults to the SI5351 chip , as that is already the default.

But a "makefile" -DBOARD_CLOCK_GEN=MS5351_CLOCK_GEN compile flag would change that.

I just tried to build with this "make mod" ifeq ($(TARGET),F303) UDEFS = -DARM_MATH_CM4 -DVERSION=\"$(VERSION)\" -DNANOVNA_F303 -DBOARD_CLOCK_GEN=MS5351_CLOCK_GEN

And it builds ... Haven't tried it on the VNA yet

Carsten

Bingo600 commented 1 year ago

Well i went all in , and changed the makefile too.

New full diff against head (as of today 2023-01-14)

*** Just changes to Makefile , compared with previous diff.

Added some Hardware version sections in the makefile

Now you can skip the TARGET= .....

And use HW_VERSION instead - I only have a version where HW version: 4.3_MS is printed on the backside (MS5351 Clock Gen). But i know from the forum that HW version: 4.3 is printed if the board has a Si5351.

I assume (hope) the H versions have either HW version: H or HW version: H_MS printed on the back. Then the user just needs to look at the back , in order to see what HW_VERSION to specify for the correct build.

I made selections for these Hardware versions in the makefile , and still made a "make" wo TARGET or HW_VERSION "default" generate for the H/Si5351 The new hw sections automatically sets the correct TARGET= , so just the HW_VERSION is needed.

New make hw type flags

"No" HW_VERSION specified , set HW_VERSION=H

HW_VERSION=H - Build for a Model H w. Si5351 HW_VERSION=H_MS - Build for a Model H w. MS5351 HW_VERSION=4.3 - Build for a Model H4 w. Si5351 HW_VERSION=4.3_MS - Build for a Model H4 w. Si5351

Ie. make clean; HW_VERSION=H make -j make clean; HW_VERSION=H_MS make -j make clean; HW_VERSION=4.3 make -j make clean; HW_VERSION=4.3_MS make -j

I also changed the generated output files to reflect the HW_VERSION built for, new "binaries" are prefixed w : NanoVNA-$(HW_VERSION)

Ie. for the HW_VERSION=H_MS the binary outout would be: NanoVNA-H_MS.bin

Hope it's usable (maybe even committed) , at least it gives an idea of how i would do it.

Carsten

patch-hw-variant.diff.txt hw-variant.zip

Bingo600 commented 1 year ago

Ohh If you build the fw , and then later do a make flash wo. specifying HW_VERSION , that matches the previous HW_VERSION the fw was build for , it will fail as it would look for the default binaries (H). Solution is to specify the HW_VERSION on the make flash line : Ie. make HW_VERSION=4.3_MS flash

Or just do all on one line make clean; HW_VERSION=4.3_MS make -j flash

Bingo600 commented 1 year ago

I just tried make clean; HW_VERSION=4.3_MS make -j flash

And now MODE shows MS5251 on first boot/reset.

Bingo600 commented 1 year ago

Improvements

1: Detect if user specifies invalid values for HW_VERSION

2: On make , and later make flash wo specifying same HW_VERSION on both commands Catch the missing binfile mismatch , before the compiler/dfu "Barfs" at user.

Bingo600 commented 1 year ago

Solved above improvement 1:

And decided to "grep" the defined version number from main.c , and use it in the generated object names. For now the objectname versioning only works for linux , as i don't have an OSX to test with. There is disabled code in makefile for OSX too.

If on linux , where we do grep for the version in main.c Object names now looks like this : FW-v1.2.19-Model-4.3_MS.bin else Object names now looks like this : FW-Model-4.3_MS.bin

Hope it can be used NB: I'm a git noob , used to use subversion , i can make a diff , but don't expect me to do pull requests etc ...

patch-hw-variant.diff.txt patch-hw-variant.zip

Bingo600 commented 1 year ago

@Ho-Ro Didn't you ask for some kind of object file versioning etc .... Have a look at the latest makefile above

Ho-Ro commented 1 year ago

Hi Carsten, I rearranged your changes a little bit in a new branch. Just clone my repo, go to this directory and init the git submodules (do this once).

git clone https://github.com/Ho-Ro/NanoVNA-D NanoVNA-D_Ho-Ro
cd NanoVNA-D_Ho-Ro
git submodule update --init --recursive

Now checkout my test branch make_SI_MS_FW and do your tests

git checkout make_SI_MS_FW
Bingo600 commented 1 year ago

@Ho-Ro Hi Martin The reason i made HW_VERSION=4.3 instead of H4 Was that the HW_VERSION=4.3 is actually printed on the backside of the Box (see pict)

So user instruction would be the "easy" : Look for the HW Version on the backside , and write what it says

H4-MS-Back

Neat rewrite of the Makefile :+1: I like your object naming layout better than mine ...

How do you like the makefile main.c - version grep thingy ... A bit of a kludge but it was what there were to work with.

Re: Testing the new Makefile I tested all combi's , all worked. And tested a few illegal HW_VERSION too - Fails nicely

And finally did this one w. my H4_MS in DFU mode ... Build & Flashed fine (MS5351 was set on first boot) make clean; HW_VERSION=H4_MS make -j flash

Well also did this one in DFU mode ... Build & Flashed fine (Si5351 was set on first boot) make clean; HW_VERSION=H4 make -j flash

Re: HW_VERSION=H4 vs HW_VERSION=4.3 I really don't care , it all depends on what a less experienced user would understand better It would be easy to say : Look at the back

Attached is a "compromise" :-) That accepts both H4 & 4.3 or H4_MS & 4.3_MS
It will name the objectfile according to the passed argument.

Carsten Makefile.diff.txt Makefile.diff.zip

DiSlord commented 1 year ago

Possible autodetect MS or SI variant, need read Register 0. Device Status bits [1:0] Revision ID. As i test before MS and SI have different values.

Ho-Ro commented 1 year ago

@Bingo600 I follow the development naming as used widely also by users. But with DiSlord's idea it's maybe not necessary to build two versions.

@DiSlord that's cool, I compared both data sheets (do you have an English version of MS, I've only the Chinese one) and didn't see this difference. Honestly speaking this autodetect was an idea earlier today, but I did refuse it (too fast) as not being possible.

BTW,what's your opinion about including the version in the filename, either using a.b.c or - (my favourite) - "tinySA-style", i.e. a.b.c-nnn-gxxxxxxx?

Martin

Bingo600 commented 1 year ago

@DiSlord That would be neat if the chip version could be autodetected. I had a brief look at the Si5351 DS , and the RevID[1:0] bit values aren't even specified , "just factory set"

Makes me think, the possibility of user overriding the chip id in the GUI should be kept. And maybe add a flag, that specifies a user override has been set, that way the firmware can detect the chip id. But if there is a user override, then that should have priority.

@Ho-Ro I think the english DS for the MS5351 is here , quite unusable (lacking any register info) https://www.qrp-labs.com/images/synth/ms5351m.pdf

Re: Naming I agree that calling the 4.3 for H4 might be optimal , as that is the "well known sales name". I give in :-) ...

Carsten

Ho-Ro commented 1 year ago

the english DS for the MS5351 is here , quite unusable

I fully agree with you, even [QRP Labs] (https://www.qrp-labs.com/synth/ms5351m.html) does not go deep enough into the real measurable differences between SI and MS. But since I don't have the HW available on an eval board on my desk, I won't be able to pursue this point further, I'll leave it to the HW gurus @DiSlord or @hugen79 to figure something out.

Bingo600 commented 1 year ago

@Ho-Ro Is there any chance that our changes would be merged into the DiSlord main ? It wouldn't do any harm ....

If not , then what about at least the makefile changes ?? HW_VERSION and Version "grep"