Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rc.board_param init file to specify custom board param save location #20805

Closed
wants to merge 1 commit into from

Conversation

PetervdPerk-NXP
Copy link
Member

@PetervdPerk-NXP PetervdPerk-NXP commented Dec 20, 2022

MTD_PARAMETERS can be freely defined as any path in the board definition.
But the rcS startup script still has to check all paths manually for all boards.

Current status:

  • /fs/mtd_params
  • /dev/eeeprom0
  • /mnt/qspi/params

Add board specific rc.board_param file where the board can specify the parameter save file.

Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the intent of how this is supposed to work.

@PetervdPerk-NXP
Copy link
Member Author

So what's the intent how it's supposed to work?
When there's a entry MTD_PARAMETERS defined you've got a path to store parameters locally right?
Problem is that it's only limited to the PX4 FRAM storage MTD driver, yet can also store it to a dedicated HW pheripheral S32K1XX EEPROM or a mounted filesystem, for S32K3 LittleFS @ /mnt/qspi

@davids5
Copy link
Member

davids5 commented Dec 20, 2022

All the paths are legacy V2 or flexible with the mtd.cpp in the board fille.

On can define mtd.cpp in the board support if the default v2 mtd behavior (that is FRAM only) is not enough. Then the manifest (mft) can be queried See

https://github.com/PX4/PX4-Autopilot/blob/main/src/systemcmds/mft/mft.cpp

The usage in RC is determining if the Keys Subject and value are present.

The rest of the mounts are done by the system. The mtd command can be used to. https://github.com/PX4/PX4-Autopilot/blob/main/src/systemcmds/mtd/mtd.cpp

I think your use case would be served by an mtd.cpp file and follow the naming convention for param. If not we can hop on a call and work it out.

davids5
davids5 previously approved these changes Dec 20, 2022
Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok this makes sense. Let's see if HW CI passes

@dk7xe
Copy link
Contributor

dk7xe commented Dec 20, 2022

@PetervdPerk-NXP please rebase! See #20794 (comment)

davids5
davids5 previously approved these changes Dec 20, 2022
Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - What has CI so bent out of shape?

@PetervdPerk-NXP
Copy link
Member Author

@davids5 Previous proposal broke backup/fallback mechanism.
Now I'm using a board specific override mechanism, is also a bit cleaner I think.

@PetervdPerk-NXP PetervdPerk-NXP changed the title Param when select is MTD_PARAMETERS query mtd for param location rc.board_param init file to specify custom board param save location Dec 21, 2022
Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PetervdPerk-NXP - good catch and solution

@dagar
Copy link
Member

dagar commented Dec 21, 2022

After the parameter server is in place (#20773) I was thinking of pushing more of this into kconfig and getting away from the brittle init script configuration of the parameter system. Let me know if you have any input.

@dk7xe
Copy link
Contributor

dk7xe commented Dec 22, 2022

Logfile of driving around indoors yesterday - https://logs.px4.io/plot_app?log=0d0ab8ed-2c00-4c19-a6d1-8931975f00ae

@junwoo091400
Copy link
Contributor

@dagar but for now it seems like this would be a good change to have in (we can accommodate to parameter server later), what do you think?

@junwoo091400
Copy link
Contributor

During today's maintainers call, @dagar pointed out that directory where the flight log gets saved to should rather be a parameter to the user to set.

But this would be a good first step into having a more flexible data path in px4. Could we get this in since it's already approved?

@junwoo091400 junwoo091400 added the Maintainers call Add items that should be discussed in the weekly maintainers call! label Jul 25, 2023
@DronecodeBot
Copy link

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-community-q-a-july-26-2023/33335/1

@junwoo091400 junwoo091400 removed the Maintainers call Add items that should be discussed in the weekly maintainers call! label Aug 1, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants