Adding new cameras, applying patches into trunk (with source code prepared) - page 154 - General Discussion and Assistance - CHDK Forum

Adding new cameras, applying patches into trunk (with source code prepared)

  • 1611 Replies
  • 429926 Views
Advertisements
No purpose for CHDK.

I believe providing a user-friendly installer for CHDK ultimately serves CHDK.

Quote
If I may remind you, you are dealing with people who have thousands of hours of experience with CHDK.
Be polite or risk getting banned.

I meant no offense or disrespect. I see nothing impolite in questioning design choices.

Nevertheless, if any of my previous remarks had hurt your feelings, I hereby offer my apology.
Author of CHIMP, Canon Hack Installation and Management Platform

*

Offline philmoz

  • *****
  • 3116
    • Photos
For a dual partition card, this appears to change the 'size' value returned by luaCB_get_partitionInfo from the size of the large partition where photos will be saved, to the size of the small (bootable) partition.


Phil.

That's partially correct. That value has nothing to do with partition info (it doesn't even return the size of a partition). Anyway, this should "fix" that.


I'm not comfortable with changing the behaviour of get_partitionInfo by adding a magic parameter, and further complicating the already messy partition code in wrappers.c.


I think it would be better to add a new Lua function to retrieve all of the SD card related information as a single table.
This would include all the data from get_partitionInfo, plus:
- free space on the photo storage partition
- sector size (although this is currently a constant I think it is worth including for completeness)
- an array of partition data, for each partition include a table with status, type, start LBA sector and sector count values


Corresponding functions in wrappers.c would return this information to the Lua code.


This way any existing script code is unaffected, and new scripts can access all of the SD card details more easily.
It can also be extended with additional data if needed.


Although this could be done by simply extending the table returned from get_partitionInfo, I think it is also worth renaming the fields to make it clearer what they are (doing this in get_partitionInfo makes it incompatible).
Field name changes could be (suggestions welcome):
- count --> partition_count
- active --> bootable_partition
- type --> bootable_partition_type
- size --> photo_partition_sizeMB


I have almost completed testing this, if there are no changes/objections I will commit over the weekend.


Phil.

CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)

Great idea overall, but may I politely offer my remarks?

Although this could be done by simply extending the table returned from get_partitionInfo, I think it is also worth renaming the fields to make it clearer what they are (doing this in get_partitionInfo makes it incompatible).

I'm not sure if field renaming alone warrants an entirely new interface, but I suppose that's a matter of personal taste.

Quote
Field name changes could be (suggestions welcome):
- count --> partition_count
- active --> bootable_partition
- type --> bootable_partition_type
- size --> photo_partition_sizeMB

I'd suggest boot_partition and boot_partition_type, since "bootable" could be interpreted as signifying the presence of the bootflag.
« Last Edit: 29 / June / 2017, 18:30:12 by dmitrys »
Author of CHIMP, Canon Hack Installation and Management Platform

*

Offline philmoz

  • *****
  • 3116
    • Photos
Great idea overall, but may I politely offer my remarks?

Although this could be done by simply extending the table returned from get_partitionInfo, I think it is also worth renaming the fields to make it clearer what they are (doing this in get_partitionInfo makes it incompatible).

I'm not sure if field renaming alone warrants an entirely new interface, but I suppose that's a matter of personal taste.
I see three options:
- keep existing field names and extend get_partitionInfo. Main issue is that the field names are confusing.
- rename fields and extend get_partitionInfo. May break existing scripts.
- leave get_partitionInfo alone and add new function with new interface. Doesn't break anything and allows us to create a more intuitive interface.

I think the 3rd option is preferable, if we can decide on good field names.

Quote
Quote
Field name changes could be (suggestions welcome):
- count --> partition_count
- active --> bootable_partition
- type --> bootable_partition_type
- size --> photo_partition_sizeMB

I'd suggest boot_partition and boot_partition_type, since "bootable" could be interpreted as signifying the presence of the bootflag.

Either way may cause some level of confusion. If partitions have been swapped on a dual-partition card then this is no longer the 'boot' partition; but it is still potentially the auto-bootable partition (although may not be). This is why I chose 'bootable' rather than 'boot'. Perhaps something else entirely might be needed; but I can't think of an alternative.

Phil.

CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)


Either way may cause some level of confusion. If partitions have been swapped on a dual-partition card then this is no longer the 'boot' partition; but it is still potentially the auto-bootable partition (although may not be). This is why I chose 'bootable' rather than 'boot'. Perhaps something else entirely might be needed; but I can't think of an alternative.

"service partition" maybe?
Author of CHIMP, Canon Hack Installation and Management Platform

*

Offline reyalp

  • ******
  • 12002
I see three options:
- keep existing field names and extend get_partitionInfo. Main issue is that the field names are confusing.
- rename fields and extend get_partitionInfo. May break existing scripts.
- leave get_partitionInfo alone and add new function with new interface. Doesn't break anything and allows us to create a more intuitive interface.
FWIW, we do have @chdk_version to handle #2. If it's only a matter of more/renamed fields, a <=1.4 compatibility wrapper would be trivial. I'm pretty agnostic whether this is better, though all else being equal I'd rather not proliferate functions that return similar info.
Don't forget what the H stands for.

*

Offline philmoz

  • *****
  • 3116
    • Photos
I see three options:
- keep existing field names and extend get_partitionInfo. Main issue is that the field names are confusing.
- rename fields and extend get_partitionInfo. May break existing scripts.
- leave get_partitionInfo alone and add new function with new interface. Doesn't break anything and allows us to create a more intuitive interface.
FWIW, we do have @chdk_version to handle #2. If it's only a matter of more/renamed fields, a <=1.4 compatibility wrapper would be trivial. I'm pretty agnostic whether this is better, though all else being equal I'd rather not proliferate functions that return similar info.


That's another option, I forgot about the version wrappers. If the changes are applied to both 1.4 & 1.5, then it probably only needs a wrapper in wrap13.lua.


Phil

CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)

*

Offline reyalp

  • ******
  • 12002
That's another option, I forgot about the version wrappers. If the changes are applied to both 1.4 & 1.5, then it probably only needs a wrapper in wrap13.lua.
I was thinking wrapper for <= 1.4, with the new function only existing in 1.5. Changing the interface for scripts using @chdk_version 1.4 would not be good, since the "stable" in stable release is mostly the interfaces.

Making this change only for 1.5 doesn't help dmitrys problem though, which would argue for a separate function. Even that's pushing the definition of bug fixes only. We could still replace the old interface with a compatibility wrapper in 1.5
Don't forget what the H stands for.


*

Offline philmoz

  • *****
  • 3116
    • Photos
That's another option, I forgot about the version wrappers. If the changes are applied to both 1.4 & 1.5, then it probably only needs a wrapper in wrap13.lua.
I was thinking wrapper for <= 1.4, with the new function only existing in 1.5. Changing the interface for scripts using @chdk_version 1.4 would not be good, since the "stable" in stable release is mostly the interfaces.

Making this change only for 1.5 doesn't help dmitrys problem though, which would argue for a separate function. Even that's pushing the definition of bug fixes only. We could still replace the old interface with a compatibility wrapper in 1.5

Damn compatibility!

Agree, technically not a bug fix; but not a huge change either.

I'm thinking:
- for 1.4 (release) - add new function with new interface for CHIMP to use, keep existing get_partitionInfo as is.
- for 1.5 - add new function with new interface, remove existing get_partitionInfo from luascript.c and add to wrap14.lua.

Will this work or am I still missing something?

Phil.

CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)

Will this work or am I still missing something?

This will probably work, but will be of little to no use as far as CHIMP is concerned, due to the fact that the latter still has to support existing versions. This also applies to my patch, albeit with one distinction: mine was a relatively small and isolated change.
Author of CHIMP, Canon Hack Installation and Management Platform

 

Related Topics