From c769c1724c133a25530f88860138cef6cb962792 Mon Sep 17 00:00:00 2001 From: AI Dev Date: Sat, 29 Aug 2026 06:17:12 +0000 Subject: [PATCH 1/2] Ask which chip it is, and which layout goes on it A camera with a ruined overlay could not be revived by a full reflash. Its flash was 16MB, the 8MB entry had been chosen in the wizard, and the instructions erased 0x0..0x800000 and stopped. Every OpenIPC mtdparts ends `-(rootfs_data)`, meaning to the end of the device, so the overlay ran to 0x1000000 and the half above the erase survived. /init mounts a jffs2 it finds and only reformats when that mount fails, so the old, broken overlay came back every boot. The reporter got out of it by generating the 8MB page for its image, the 16MB page for its `sf erase 0x0 0x1000000`, and editing the filename by hand. The menu was one field doing two jobs: how big the chip is, and how it is carved up. Those are not always the same answer -- nor32m was already the exception, rewritten to the nor16m command set because no bootloader upstream defines mtdpartsnor32m. So split them. The flash type stays the chip and keeps its permanent-link key; a second menu picks the layout and defaults to the chip's own, which is what every combination the form could produce before this already had. The assembled image is the size of the chip whatever layout goes inside it, so the erase spans the whole part and there is nowhere for a stale overlay to survive. Everything that used to key on the chip and meant the layout now says so: FlashLayout's offsets, the uk/ur/set macro names, and the rule that keeps Ultimate off a 5120KB rootfs partition -- which is the layout's doing and not the chip's, so it now applies to the 8MB layout on a 32MB part too. Images already cached keep their names: the layout only enters the filename when it is not the one the chip would have had anyway. --- app/controllers/cameras/socs_controller.rb | 71 ++++++++++++++--- app/helpers/installation_helper.rb | 12 ++- app/helpers/selects_helper.rb | 7 ++ app/models/camera.rb | 65 ++++++++++++++- app/models/firmware.rb | 50 ++++++++++-- app/views/cameras/socs/show.html.erb | 57 ++++++++++++-- app/views/cameras/socs/update.html.erb | 11 ++- config/locales/activemodel.en.yml | 1 + config/locales/activemodel.ru.yml | 1 + config/locales/activemodel.zh.yml | 1 + config/locales/activerecord.en.yml | 1 + config/locales/activerecord.ru.yml | 1 + config/locales/activerecord.zh.yml | 1 + config/locales/en.yml | 5 ++ config/locales/ru.yml | 5 ++ config/locales/zh.yml | 5 ++ test/controllers/socs_controller_test.rb | 92 ++++++++++++++++++++-- test/models/camera_test.rb | 68 +++++++++++++++- test/models/firmware_test.rb | 61 +++++++++++++- 19 files changed, 473 insertions(+), 42 deletions(-) diff --git a/app/controllers/cameras/socs_controller.rb b/app/controllers/cameras/socs_controller.rb index 1d9b4e88..9567adda 100644 --- a/app/controllers/cameras/socs_controller.rb +++ b/app/controllers/cameras/socs_controller.rb @@ -86,6 +86,7 @@ def update @camera.camera_mac_address = permitted_params[:camera_mac_address].to_s.downcase.gsub('-', ':') @camera.server_ip_address = permitted_params[:server_ip_address] @camera.flash_type = permitted_params[:flash_type] + @camera.partition_layout = permitted_params[:partition_layout] @camera.firmware_version = permitted_params[:firmware_version] @camera.network_interface = permitted_params[:network_interface] @camera.sd_card_slot = permitted_params[:sd_card_slot] @@ -112,13 +113,20 @@ def update # query string and the commands. @camera.flash_type = @camera.soc.default_flash_chip unless @camera.flash_type.in?(Camera::FLASH_CHIP) - # to handle nor32m size still using nor16m command. After the default - # above, not before: the commands name the chip, so reading the flash type - # first left the page telling a 16MB camera to `run urnor16m` and then - # erasing from the 8MB overlay offset, 733,184 bytes into what it had just - # written. That is the failure #60 described, by another route. - @flash_type_command = @camera.flash_type - @flash_type_command = 'nor16m' if @camera.flash_type.eql?('nor32m') + # The bootloader macros are named after the layout, not the chip. This + # used to be the flash type with `nor32m` rewritten to `nor16m`, which is + # the same answer for every combination the menu could then produce -- + # there is no mtdpartsnor32m anywhere upstream, so a 32MB part has always + # worn the 16MB layout. Camera#partition_layout says it directly now, and + # says it for the 8MB-layout-on-a-larger-chip case too. + # + # After the flash type has settled, not before: the layout defaults to + # the chip's own, so reading it first left the page telling a 16MB camera + # to `run urnor16m` and then erasing from the 8MB overlay offset, 733,184 + # bytes into what it had just written. That is the failure #60 described, + # by another route. + warn_if_layout_changed permitted_params[:partition_layout] + @flash_type_command = @camera.partition_layout if @vendor.name.eql?("SigmaStar") && @camera.flash_type.eql?("nand") render 'cameras/socs/sigmastar_nand_is_weird' @@ -152,12 +160,15 @@ def update end def download_full_image - permitted_params = params.permit(:id, :vendor_id, :flash_size, :fw_release, :flash_type) + permitted_params = params.permit(:id, :vendor_id, :flash_size, :fw_release, :flash_type, :layout) flash_size = permitted_params[:flash_size] flash_type = permitted_params[:flash_type] fw_release = permitted_params[:fw_release] @soc = Soc.find(params[:id]) - fw = Firmware.new(size: flash_size, flash_type: flash_type, release: fw_release, soc: @soc) + # An absent layout means the chip's own, which is what every link written + # before the wizard could tell the two apart meant. + fw = Firmware.new(size: flash_size, flash_type: flash_type, release: fw_release, soc: @soc, + layout: permitted_params[:layout]) fw.generate # Recorded here rather than in Firmware, because a cached image is sent # without being rebuilt and it is the sending that is worth counting. @@ -236,6 +247,7 @@ def full_list # between a key and a field and a table is where that is visible. Insertion # order is the precedence: `var` is applied first so `ver` overwrites it. PERMALINK_FIELDS = { cip: :camera_ip_address, sip: :server_ip_address, rom: :flash_type, + part: :partition_layout, var: :firmware_version, ver: :firmware_version, net: :network_interface, sd: :sd_card_slot }.freeze @@ -278,7 +290,7 @@ def narrow_to_what_the_menu_offers(camera) # published as Ultimate and nothing else, hi3516cv6xx and hi3519dv500, # keeps it, because naming a Lite tarball upstream never built is worse # than the size warning `update` will give. - return unless camera.flash_type.eql?('nor8m') && camera.firmware_version.eql?('ultimate') + return unless camera.partition_layout.eql?('nor8m') && camera.firmware_version.eql?('ultimate') return unless camera.soc.available_releases('nor').include?('lite') camera.firmware_version = 'lite' @@ -318,7 +330,7 @@ def missing_asset_message(soc) # download link for `flash_size=8&fw_release=ultimate`, and `run uknor8m; # run urnor8m` -- with no indication that none of it can work. def enforce_eight_meg_limit - return unless @camera.flash_type.eql?('nor8m') && @camera.firmware_version.eql?('ultimate') + return unless eight_meg_rootfs_with_ultimate? published = @camera.soc.available_releases('nor') # Nothing on NOR at any size is a different problem, and @@ -328,7 +340,7 @@ def enforce_eight_meg_limit if published.include?('lite') @camera.firmware_version = 'lite' - flash.now[:warning] = '8MB Flash ROM can only be flashed with Lite or FPV edition!' + flash.now[:warning] = eight_meg_warning else flash.now[:alert] = 'The Ultimate edition does not fit an 8MB flash chip, and OpenIPC publishes no Lite build ' \ @@ -337,6 +349,23 @@ def enforce_eight_meg_limit end end + # The layout, not the chip: the 8MB one gives the rootfs 5120KB wherever it + # is written, and that is what Ultimate does not fit in. + def eight_meg_rootfs_with_ultimate? + @camera.partition_layout.eql?('nor8m') && @camera.firmware_version.eql?('ultimate') + end + + # The chip when the chip is what limits them, and the layout when it is the + # layout: a 5120KB rootfs partition is a 5120KB rootfs partition whether the + # part around it is 8MB or 32MB, and on the larger ones there is something + # the reader can actually do about it. + def eight_meg_warning + return '8MB Flash ROM can only be flashed with Lite or FPV edition!' if @camera.flash_type.eql?('nor8m') + + 'The 8MB partition layout leaves 5MB for the rootfs, which only the Lite and FPV editions ' \ + 'fit. Choose the 16MB layout to install Ultimate on this chip.' + end + # Nothing published for the chip that was chosen, at any edition. # # Distinct from use_published_release!, which moves the visitor onto a @@ -352,6 +381,22 @@ def enforce_eight_meg_limit # available_releases answers the known list rather than [] when the index # cannot be read, so an unreachable index does not turn into "OpenIPC # publishes nothing for this SoC". + # The layout is refused rather than clamped in silence when the chip cannot + # hold it -- reachable only from a hand-edited query string, since the menu + # does not offer the 16MB layout on an 8MB part, but it decides where the + # rootfs is written and a page that quietly showed the other one would be + # describing a different install from the one that was asked for. + def warn_if_layout_changed(asked) + # Unrecognised is the same as unset, as it is everywhere else here, and + # NAND has one layout and no menu to choose it from. + return if @camera.nand? || !asked.in?(Camera::PARTITION_LAYOUT) + return if asked.eql?(@camera.partition_layout) + + flash.now[:warning] = + 'The 16MB partition layout needs a 16MB chip -- its rootfs alone ends at 0xD50000. ' \ + "Showing the #{@camera.layout_size}MB layout instead." + end + def warn_if_nothing_published return if @camera.soc.available_releases(@camera.flash_type_type).any? @@ -362,7 +407,7 @@ def warn_if_nothing_published def permitted_params params.require(:camera).permit( - :flash_type, :sd_card_slot, :network_interface, :camera_ip_address, + :flash_type, :partition_layout, :sd_card_slot, :network_interface, :camera_ip_address, :server_ip_address, :firmware_version, :sd_card_slot, :camera_mac_address ) end diff --git a/app/helpers/installation_helper.rb b/app/helpers/installation_helper.rb index 1f4f36ad..b3ab0542 100644 --- a/app/helpers/installation_helper.rb +++ b/app/helpers/installation_helper.rb @@ -90,10 +90,20 @@ def firmware_backup(c) def flashing_everything(c) fw_filename = Firmware.filename_for(soc_model: c.soc.model_downcase, flash_type: c.flash_type_type, - release: c.firmware_version, size: c.flash_size) + release: c.firmware_version, size: c.flash_size, + layout: c.layout_size) # The full image is exactly the size it claims on NOR and page-aligned by # construction on NAND, so ${filesize} is always a safe write length here -- # unlike the u-boot-only block below, where the binary is neither. + # + # And on NOR that size is the chip's, whatever partition layout is going + # inside it, so the erase below spans the whole part. It used to span the + # size of the layout instead, which is the same number for every + # combination the menu could produce until it grew a second field -- and + # then, for an 8MB layout on a 16MB chip, left the top half of the flash + # untouched. The overlay ends `-(rootfs_data)` and so runs to the end of + # the device: a jffs2 that survives up there is mounted on the next boot + # and the camera comes back exactly as broken as it went in. write_size = '${filesize}' text = [] text << do_not_copy_paste diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb index 2c7207c1..6b840d8f 100644 --- a/app/helpers/selects_helper.rb +++ b/app/helpers/selects_helper.rb @@ -21,6 +21,13 @@ def list_of_flash_type_sizes_for_select end end + # Both layouts, always. Which of them a chip can hold is narrowed on the page, + # beside the rule that narrows the editions -- an 8MB part cannot wear the + # 16MB layout, whose rootfs partition ends at 0xD50000. + def list_of_partition_layouts_for_select + Camera::PARTITION_LAYOUT.map { |layout| [t("flash_layout.#{layout}"), layout] } + end + # Every SoC used to be offered lite, ultimate and fabricator whatever upstream # built. The list comes from the release index now -- the union across flash # types, because the flash type is chosen in the same form without a round diff --git a/app/models/camera.rb b/app/models/camera.rb index acb88714..6b311727 100644 --- a/app/models/camera.rb +++ b/app/models/camera.rb @@ -10,6 +10,21 @@ class Camera # never has. `neo` is here because it does exist, for seven boards. FW_VERSION = %w[lite ultimate neo].freeze FLASH_CHIP = %w[nor8m nor16m nor32m nand].freeze + + # How the chip is carved up, which is a different question from how big it + # is. Every OpenIPC bootloader carries both mtdpartsnor8m and mtdpartsnor16m + # and either can be run on a part large enough to hold it, so a 16MB chip can + # perfectly well wear the 8MB layout -- and one flashed from the 8MB image + # already does. + # + # The two were a single menu entry until a camera turned up that could not be + # revived by a full reflash. Its chip was 16MB and the 8MB entry had been + # chosen, so the instructions erased 0x0..0x800000 and stopped. Every layout + # here ends `-(rootfs_data)`, meaning "to the end of the device", so the + # overlay ran to 0x1000000 and the half of it above the erase survived intact. + # /init mounts jffs2 and only reformats when that mount fails, so the old, + # broken overlay came back every time. + PARTITION_LAYOUT = %w[nor8m nor16m].freeze NET_IFACE = %w[eth wifi both].freeze SD_CARD = %w[nosd sd].freeze @@ -36,6 +51,7 @@ class Camera attr_accessor :soc_id, :needs_instruction, :flash_type, :sd_card_slot, :network_interface, :camera_ip_address, :server_ip_address, :firmware_version, :camera_mac_address, :soc, :backup_filename + attr_writer :partition_layout validates :soc_id, presence: true validates :flash_type, presence: true @@ -185,12 +201,56 @@ def firmware_version_name default: firmware_version.to_s.capitalize) end + # Which mtdparts this camera is being given. Defaults to the one that matches + # the chip, so a visitor who never opens the second menu gets exactly what + # this page has always produced, and refuses a layout the chip cannot hold -- + # the 16MB one ends at 0xD50000, which is past the end of an 8MB part. + # + # Unrecognised is the same as unset. Like every other field here it can arrive + # from a query string, and nothing calls valid? on a Camera. + def partition_layout + return 'nand' if nand? + return default_partition_layout unless @partition_layout.in?(PARTITION_LAYOUT) + return default_partition_layout unless layout_fits_chip?(@partition_layout) + + @partition_layout + end + + def default_partition_layout + return 'nand' if nand? + + flash_size <= 8 ? 'nor8m' : 'nor16m' + end + + # The 8MB layout fits anything; the 16MB one needs a 16MB part. + def layout_fits_chip?(layout) + layout.eql?('nor8m') || flash_size >= 16 + end + + def layout_size + partition_layout.eql?('nor8m') ? 8 : 16 + end + + # `nand` is not in PARTITION_LAYOUT and cannot be chosen: NAND has one layout, + # mtdpartsubi, and the menu is hidden for it. It is what partition_layout + # answers there so that the name doubles as the suffix of the bootloader + # macros -- `uknand`, `urnand`, `setnand`, and `uknor8m` and friends on NOR. + # That is also why a 32MB part has always been told to `run setnor16m`: there + # is no mtdpartsnor32m in any bootloader upstream ships. + def partition_layout_name + I18n.t("flash_layout.#{partition_layout}") + end + # The NOR numbers come from FlashLayout, which reads them off the bootloader # environment. They used to be spelled out here keyed on firmware_version, # which agreed with the bootloader only for 8MB+Lite and 16MB+Ultimate; see # FlashLayout for what that cost on 16MB. + # + # Keyed on the layout, not on the chip. The two agree for every combination + # the menu offered before it grew a second field, and the whole point of the + # second field is the ones where they do not. def nor_layout - FlashLayout.nor(flash_size) + FlashLayout.nor(layout_size) end def kernel_max_size @@ -251,6 +311,9 @@ def permalink '&sip=', server_ip_address, '&net=', network_interface, '&rom=', flash_type, + # The layout is written whether or not it differs from the chip's own, so + # a link says what it means rather than leaning on today's default. + '&part=', partition_layout, # `ver`, not `var`. This emitted `var` while show has always read `ver`, # so the edition was the one field the permanent link dropped: reopening # a link for Ultimate on a 32MB chip came back as Lite. show still diff --git a/app/models/firmware.rb b/app/models/firmware.rb index 5534c49f..60b2b4a4 100644 --- a/app/models/firmware.rb +++ b/app/models/firmware.rb @@ -12,6 +12,12 @@ class Firmware NOR_SIZES = [8, 16, 32].freeze + # The two mtdparts every OpenIPC bootloader carries. The layout is not the + # chip: an 8MB layout on a 16MB part is a real configuration, and building it + # as a chip-sized image is what lets the installation page erase the whole + # part rather than stopping at the end of a smaller one. + NOR_LAYOUTS = [8, 16].freeze + LOCK_POLL = 0.1 # Assembly of one image takes about a second. The wait exists only so that @@ -45,25 +51,41 @@ class InvalidFlashSize < StandardError; end class PayloadTooLarge < StandardError; end class LockTimeout < StandardError; end - def initialize(size: 8, flash_type: 'nor', release: 'lite', soc: nil) + def initialize(size: 8, flash_type: 'nor', release: 'lite', soc: nil, layout: nil) super() @soc = soc @size = size.to_i @flash_type = flash_type @release = release + # Blank as well as nil, because this arrives from a query string and an + # older permanent link carries no layout at all. + @layout = layout.to_s.strip.empty? ? self.class.natural_layout(@size) : layout.to_i + end + + # The layout a chip of this size wears unless told otherwise, and the one + # every image built before there was a choice was built with. + def self.natural_layout(size) + size.to_i <= 8 ? 8 : 16 end # Built here rather than in the view so the TFTP command on the installation # page and the file the download actually produces cannot drift apart. - def self.filename_for(soc_model:, flash_type:, release:, size:) + # + # The layout only enters the name when it is not the chip's own. Every image + # already on disk was built before the two could differ, so leaving the usual + # combinations spelled as they always were keeps that cache valid -- and keeps + # the file a visitor downloads named after the chip they told us about. + def self.filename_for(soc_model:, flash_type:, release:, size:, layout: nil) return "openipc-#{soc_model}-nand-#{release}.bin" if flash_type.to_s.eql?('nand') - "openipc-#{soc_model}-nor-#{release}-#{size}mb.bin" + layout = natural_layout(size) if layout.to_s.strip.empty? + suffix = layout.to_i.eql?(natural_layout(size)) ? '' : "-parts#{layout.to_i}m" + "openipc-#{soc_model}-nor-#{release}-#{size}mb#{suffix}.bin" end def filename @filename ||= self.class.filename_for(soc_model: @soc.model_downcase, flash_type: @flash_type, - release: @release, size: @size) + release: @release, size: @size, layout: @layout) end def filepath @@ -100,6 +122,7 @@ def flash_size # for a name that has been aliased away is exactly what should not be served. def generate validate_size! + validate_layout! build_if_needed rescue ReleaseCache::Unavailable => e raise unless usable? @@ -283,6 +306,19 @@ def validate_size! raise InvalidFlashSize, "unsupported NOR flash size #{@size}MB (expected #{NOR_SIZES.join(', ')})" end + # Same reasoning as validate_size!, for the same reason: the layout is a + # request parameter too, and it decides the offsets everything is written at. + # A layout larger than the chip is refused rather than clamped -- the 16MB one + # puts the rootfs at 0x350000 with 10240KB to fill, which does not exist on an + # 8MB part, and silently substituting the other one would hand back an image + # that does not match the name it was asked for. + def validate_layout! + return if nand? + return if NOR_LAYOUTS.include?(@layout) && @layout <= @size + + raise InvalidFlashSize, "unsupported NOR partition layout #{@layout}MB on a #{@size}MB chip" + end + # IO.binwrite past the end of a file grows it rather than failing, so a part # too big for its slot produced an image larger than the chip it names # instead of an error. openipc-hi3516ev200-nor-ultimate-8mb.bin was found on @@ -389,9 +425,11 @@ def rootfs_member end # Same table the installation page renders from, so the image and the - # instructions cannot describe different partition layouts. + # instructions cannot describe different partition layouts. Keyed on the + # layout rather than the size, which are the same thing for every image built + # before the wizard could tell them apart. def nor_layout - @nor_layout ||= FlashLayout.nor(@size) + @nor_layout ||= FlashLayout.nor(@layout) end def kernel_offset diff --git a/app/views/cameras/socs/show.html.erb b/app/views/cameras/socs/show.html.erb index 7e8d327d..21a1d5d4 100644 --- a/app/views/cameras/socs/show.html.erb +++ b/app/views/cameras/socs/show.html.erb @@ -81,6 +81,11 @@
<%= f.select :flash_type, list_of_flash_type_sizes_for_select %> + <%# Hidden for NAND, which has one layout -- mtdpartsubi -- and %> + <%# nothing to choose. The script below does the hiding. %> +
+ <%= f.select :partition_layout, list_of_partition_layouts_for_select %> +
<%= f.select :firmware_version, list_of_firmware_versions_for_select, {}, data: { availability: @camera.soc.release_availability.to_json } %> @@ -109,8 +114,13 @@ document.querySelector('#camera_firmware_version').dataset.availability ); - // Flash size still constrains the edition independently of what exists: - // an 8MB part cannot hold Ultimate whether or not it is published. + // The partition layout still constrains the edition independently of what + // exists: a 5120KB rootfs partition cannot hold Ultimate whether or not it + // is published. + // + // Keyed on the layout rather than the chip, which is the same thing until + // someone asks for the 8MB layout on a larger part -- and then it is the + // partition that decides, not the chip around it. // // 16MB used to be listed here too, and that was wrong. Ultimate is built // for 16MB and for nothing else -- all 28 *_ultimate_defconfig files @@ -120,21 +130,50 @@ // exactly the hardware it targets. With the entry gone, allowedEditions // falls through to what the release index says is published, which is the // rule: both editions on 16MB where an Ultimate image exists. - const sizeLimits = {nor8m: ['lite']}; + const layoutLimits = {nor8m: ['lite']}; // What to land on when the current choice is no longer available. const preferred = {nor32m: 'ultimate', nand: 'ultimate'}; - function allowedEditions(chip) { + // Which layouts a chip can wear. The 16MB one puts 10240KB of rootfs at + // 0x350000 and so ends at 0xD50000, past the end of an 8MB part; the 8MB + // one fits anything, and on a larger chip it simply leaves rootfs_data -- + // which every mtdparts ends with `-`, meaning to the end of the device -- + // that much bigger. NAND has mtdpartsubi and no choice to make. + function allowedLayouts(chip) { + if (chip.startsWith('nand')) return []; + return chip === 'nor8m' ? ['nor8m'] : ['nor8m', 'nor16m']; + } + + // Narrow the layout menu to the chip, and answer what is now selected. + // Landing on the last allowed entry rather than the first makes the + // chip's own layout the default, which is what this form produced when + // the two were a single field. + function checkPartitionLayout() { + const el = document.querySelector('#camera_partition_layout'); + const chip = document.querySelector('#camera_flash_type').value; + const allowed = allowedLayouts(chip); + + document.querySelector('#partition-layout-field').hidden = allowed.length === 0; + for (let i = 0; i < el.options.length; i++) { + const o = el.options.item(i); + o.disabled = !allowed.includes(o.value); + } + if (allowed.length && !allowed.includes(el.value)) el.value = allowed[allowed.length - 1]; + + return el.value; + } + + function allowedEditions(chip, layout) { const published = availability[chip.startsWith('nand') ? 'nand' : 'nor'] || []; - const limit = sizeLimits[chip]; + const limit = layoutLimits[layout]; if (!limit) return published; - // The size rule narrows what exists; it must not empty the menu. No SoC + // The layout rule narrows what exists; it must not empty the menu. No SoC // listed today can hit this -- every board with a NOR build has a Lite // one -- but hi3516cv6xx and hi3519dv500 are built only as Ultimate, so // the first of those to be added here would otherwise offer nothing at - // all on 8M and 16M. Offering what exists and letting the server answer + // all on the 8MB layout. Offering what exists and letting the server answer // PayloadTooLarge, which it does with its own message, beats a menu with // every entry greyed out and no explanation. const fits = published.filter(v => limit.includes(v)); @@ -156,9 +195,10 @@ function checkFlashSize() { useAnOfferedFlashType(); + const layout = checkPartitionLayout(); const el = document.querySelector('#camera_firmware_version'); const chip = document.querySelector('#camera_flash_type').value; - const allowed = allowedEditions(chip); + const allowed = allowedEditions(chip, layout); for (let i = 0; i < el.options.length; i++) { const o = el.options.item(i); @@ -192,5 +232,6 @@ document.querySelector('#generate-mac-address').addEventListener('click', generateMacAddress); document.querySelector('#camera_flash_type').addEventListener('change', checkFlashSize); + document.querySelector('#camera_partition_layout').addEventListener('change', checkFlashSize); checkFlashSize(); diff --git a/app/views/cameras/socs/update.html.erb b/app/views/cameras/socs/update.html.erb index 943cac6a..a8b1ad1d 100644 --- a/app/views/cameras/socs/update.html.erb +++ b/app/views/cameras/socs/update.html.erb @@ -14,6 +14,13 @@

<%= t('firmware.installation.title') %> (<%= @camera.firmware_version_name %>)

<%= t('firmware.installation.subtitle', soc_name: @camera.soc.full_name, flash_type: @camera.flash_type_name) %>

+ <%# Which mtdparts these commands assume. Worth saying out loud: it is a %> + <%# separate choice from the chip now, and the erase below spans the chip %> + <%# whichever layout goes inside it. %> + <% unless @camera.nand? %> +

<%= t('firmware.installation.layout_note', + layout: @camera.partition_layout_name) %>

+ <% end %>

<%= link_to t('firmware.installation.permanent_link'), @camera.permalink %>

@@ -53,8 +60,8 @@
-
<%= link_to t('firmware.installation.flashing_full.link', name: @camera.firmware_version.to_s.titleize), download_full_image_cameras_vendor_soc_path(@camera.soc.vendor, @camera.soc, fw_release: @camera.firmware_version, flash_size: @camera.flash_size, flash_type: @camera.flash_type_type) %>
-

for <%= @camera.soc.full_name %> with <%= @camera.flash_size %>MB <%= @camera.flash_type_type.upcase %> flash

+
<%= link_to t('firmware.installation.flashing_full.link', name: @camera.firmware_version.to_s.titleize), download_full_image_cameras_vendor_soc_path(@camera.soc.vendor, @camera.soc, fw_release: @camera.firmware_version, flash_size: @camera.flash_size, flash_type: @camera.flash_type_type, layout: @camera.layout_size) %>
+

for <%= @camera.soc.full_name %> with <%= @camera.flash_size %>MB <%= @camera.flash_type_type.upcase %> flash<%= ", #{@camera.layout_size}MB partitions" unless @camera.nand? || @camera.layout_size == @camera.flash_size %>

<%= t('firmware.installation.flashing_full.info') %>

<% if @camera.nand? %>

<%= t('firmware.installation.flashing_full.nand_caveat') %>

diff --git a/config/locales/activemodel.en.yml b/config/locales/activemodel.en.yml index 4ab46cb7..58598dd0 100644 --- a/config/locales/activemodel.en.yml +++ b/config/locales/activemodel.en.yml @@ -8,6 +8,7 @@ en: firmware_version: Firmware version flash_type: Type and size of flash memory chip network_interface: Network interface + partition_layout: Partition layout sd_card_slot: SD card slot server_ip_address: TFTP server IP address soc_id: System-on-Chip diff --git a/config/locales/activemodel.ru.yml b/config/locales/activemodel.ru.yml index 2e745319..6517fb34 100644 --- a/config/locales/activemodel.ru.yml +++ b/config/locales/activemodel.ru.yml @@ -8,6 +8,7 @@ ru: firmware_version: Версия прошивки flash_type: Тип и размер микросхемы флэш-памяти network_interface: Сетевые интерфейсы + partition_layout: Разметка флэш-памяти на разделы sd_card_slot: Слот для SD-карты server_ip_address: IP-адрес TFTP-сервера soc_id: Система-на-чипе diff --git a/config/locales/activemodel.zh.yml b/config/locales/activemodel.zh.yml index 222f87e1..66638aa9 100644 --- a/config/locales/activemodel.zh.yml +++ b/config/locales/activemodel.zh.yml @@ -8,6 +8,7 @@ zh: firmware_version: 固件版本 flash_type: 闪存芯片类型和容量大小 network_interface: 网络接口 + partition_layout: 分区布局 sd_card_slot: SD卡槽 server_ip_address: TFTP服务器IP地址 soc_id: 芯片型号 diff --git a/config/locales/activerecord.en.yml b/config/locales/activerecord.en.yml index 510b4058..55dbde17 100644 --- a/config/locales/activerecord.en.yml +++ b/config/locales/activerecord.en.yml @@ -19,6 +19,7 @@ en: help: camera: flash_type: If not sure, select NOR 8M. + partition_layout: Follows the chip unless you change it. The 8MB layout on a larger chip leaves the rest of the flash to the overlay, but only fits the Lite edition. models: snapshot: Snapshot soc: SoC diff --git a/config/locales/activerecord.ru.yml b/config/locales/activerecord.ru.yml index 38b3517a..0b86e9d9 100644 --- a/config/locales/activerecord.ru.yml +++ b/config/locales/activerecord.ru.yml @@ -19,6 +19,7 @@ ru: help: camera: flash_type: Если вы не уверены, выберите NOR 8M. + partition_layout: По умолчанию соответствует микросхеме. Разметка 8 МБ на более крупной микросхеме отдаёт остаток флэш-памяти под оверлей, но вмещает только редакцию Lite. models: snapshot: Снимок soc: SoC diff --git a/config/locales/activerecord.zh.yml b/config/locales/activerecord.zh.yml index b9ff32da..2a6faff6 100644 --- a/config/locales/activerecord.zh.yml +++ b/config/locales/activerecord.zh.yml @@ -19,6 +19,7 @@ zh: help: camera: flash_type: 如果不确定，请选择 NOR 8M。 + partition_layout: 默认与芯片容量一致。在更大的芯片上使用 8MB 分区布局会把余下的空间留给 overlay，但只能容纳 Lite 版本。 models: snapshot: 快照 soc: 芯片型号 diff --git a/config/locales/en.yml b/config/locales/en.yml index 73ef22e5..91fbcc22 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -97,6 +97,7 @@ en: info: Download bootloader binary file and save it to the root directory of your TFTP server. link: Download OpenIPC U-Boot binary file title: Flash OpenIPC U-Boot + layout_note: 'Partition layout: %{layout}. The image below covers the whole chip, so flashing it clears the overlay as well.' permanent_link: Permanent link to this configuration subtitle: on a camera with %{soc_name} SoC and %{flash_type} SPI flash chip success: @@ -117,6 +118,10 @@ en: nor16m: NOR 16M nor32m: NOR 32M nor8m: NOR 8M + flash_layout: + nand: UBI layout + nor16m: 16MB (10MB rootfs) + nor8m: 8MB (5MB rootfs) go: Go footer: column_community: Community diff --git a/config/locales/ru.yml b/config/locales/ru.yml index 3c10dc20..b064e9cf 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -99,6 +99,7 @@ ru: info: Загрузите двоичный файл загрузчика и сохраните его в корневой каталог вашего TFTP-сервера. link: Скачать бинарный файл OpenIPC U-Boot title: Прошивка U-Boot OpenIPC + layout_note: 'Разметка на разделы: %{layout}. Образ ниже занимает всю флэш-память целиком, поэтому его прошивка стирает и оверлей.' permanent_link: Постоянная ссылка на эту конфигурацию subtitle: на камере с %{soc_name} SoC и флэш-чипом %{flash_type} SPI success: @@ -119,6 +120,10 @@ ru: nor16m: NOR 16M nor32m: NOR 32M nor8m: NOR 8M + flash_layout: + nand: Разметка UBI + nor16m: 16 МБ (rootfs 10 МБ) + nor8m: 8 МБ (rootfs 5 МБ) go: Go footer: column_community: Сообщество diff --git a/config/locales/zh.yml b/config/locales/zh.yml index 73cc1b0d..d1889f42 100644 --- a/config/locales/zh.yml +++ b/config/locales/zh.yml @@ -97,6 +97,7 @@ zh: info: 下载引导加载程序二进制文件并将其保存到 TFTP 服务器的根目录。 link: 下载 OpenIPC U-Boot 二进制文件 title: 烧录 OpenIPC U-Boot + layout_note: '分区布局：%{layout}。下面的镜像覆盖整片闪存，因此刷写时也会清除 overlay 分区。' permanent_link: 当前配置的永久链接 subtitle: 在带有 %{soc_name} SoC 和 %{flash_type} SPI 闪存芯片的摄像机上 success: @@ -117,6 +118,10 @@ zh: nor16m: NOR 16M nor32m: NOR 32M nor8m: NOR 8M + flash_layout: + nand: UBI 分区布局 + nor16m: 16MB（rootfs 10MB） + nor8m: 8MB（rootfs 5MB） go: 进行 footer: column_community: 社区 diff --git a/test/controllers/socs_controller_test.rb b/test/controllers/socs_controller_test.rb index af821932..f63dce34 100644 --- a/test/controllers/socs_controller_test.rb +++ b/test/controllers/socs_controller_test.rb @@ -194,16 +194,94 @@ def instructable_soc(model) linux_filename: "openipc.#{model.downcase}-nor-lite.tgz") end - def submit(soc, flash_type, firmware_version: 'lite', locale: nil) + # partition_layout is left out unless a test asks for one, so the default path + # -- which is what the form sends for every chip whose layout is its own -- is + # what the rest of these tests keep exercising. + def submit(soc, flash_type, firmware_version: 'lite', partition_layout: nil, locale: nil) + camera = { flash_type:, firmware_version:, + network_interface: 'eth', sd_card_slot: 'nosd', + camera_ip_address: '192.168.1.10', + server_ip_address: '192.168.1.254', + camera_mac_address: '00:11:22:33:44:55' } + camera[:partition_layout] = partition_layout if partition_layout + put "/cameras/vendors/#{soc.vendor.to_param}/socs/#{soc.to_param}#{"?locale=#{locale}" if locale}", - params: { camera: { flash_type:, firmware_version:, - network_interface: 'eth', sd_card_slot: 'nosd', - camera_ip_address: '192.168.1.10', - server_ip_address: '192.168.1.254', - camera_mac_address: '00:11:22:33:44:55' } } + params: { camera: } assert_response :success end + # --- the chip and the layout, separately --- + + # The report this second menu exists for. A 16MB camera with a ruined overlay, + # reflashed from the 8MB entry, came back exactly as broken: the erase stopped + # at 0x800000 while rootfs_data -- `-` in every mtdparts, so to the end of the + # device -- ran to 0x1000000, and /init mounts a jffs2 it finds rather than + # reformatting it. The erase has to span the chip whatever layout goes inside. + test 'the 8MB layout on a 16MB chip erases the whole chip' do + soc = instructable_soc('TS3516EVE00') + + with_release_index(*every_edition_for(soc)) do + submit(soc, 'nor16m', partition_layout: 'nor8m') + + assert_match 'sf erase 0x0 0x1000000', response.body + assert_no_match(/sf erase 0x0 0x800000/, response.body) + # One image, laid out the 8MB way and sized for the chip it goes on. + assert_match 'openipc-ts3516eve00-nor-lite-16mb-parts8m.bin', response.body + assert_match 'layout=8', response.body + # ...and the by-parts block underneath agrees with it. + assert_match 'run uknor8m; run urnor8m', response.body + assert_match 'sf erase 0x750000 0x8b0000', response.body + end + end + + test 'a chip whose layout is its own is named and flashed exactly as before' do + soc = instructable_soc('TS3516EVE10') + + with_release_index(*every_edition_for(soc)) do + submit(soc, 'nor16m') + + assert_match 'openipc-ts3516eve10-nor-lite-16mb.bin', response.body + assert_no_match(/parts8m/, response.body) + assert_match 'sf erase 0x0 0x1000000', response.body + assert_match 'run uknor16m; run urnor16m', response.body + end + end + + test 'the 16MB layout is refused on an 8MB chip, and the page says so' do + soc = instructable_soc('TS3516EVE20') + + with_release_index(*every_edition_for(soc)) do + submit(soc, 'nor8m', partition_layout: 'nor16m') + + assert_match 'The 16MB partition layout needs a 16MB chip', response.body + assert_match 'sf erase 0x0 0x800000', response.body + assert_match 'openipc-ts3516eve20-nor-lite-8mb.bin', response.body + end + end + + # The rootfs partition is what Ultimate does not fit in, and that is the + # layout's doing rather than the chip's: 5120KB is 5120KB on a 32MB part too. + test 'Ultimate is refused by the 8MB layout even on a chip with room' do + soc = instructable_soc('TS3516EVE30') + + with_release_index(*every_edition_for(soc)) do + submit(soc, 'nor16m', firmware_version: 'ultimate', partition_layout: 'nor8m') + + assert_match 'The 8MB partition layout leaves 5MB for the rootfs', response.body + assert_match 'openipc-ts3516eve30-nor-lite-16mb-parts8m.bin', response.body + end + end + + test 'the permanent link carries the layout so it can be reopened' do + soc = instructable_soc('TS3516EVE40') + + with_release_index(*every_edition_for(soc)) do + submit(soc, 'nor16m', partition_layout: 'nor8m') + + assert_match(/part=nor8m/, response.body) + end + end + test 'a 16MB submission gets the 16MB layout, not the SoC default' do # update guarded the default_flash_chip fallback on params[:rom], which is # how `show` receives a choice and not how this action does: the form PUTs @@ -459,7 +537,7 @@ def submit(soc, flash_type, firmware_version: 'lite', locale: nil) get "/cameras/vendors/#{@vendor.to_param}/socs/#{soc.to_param}" assert_response :success - assert_match(/const sizeLimits = \{nor8m: \['lite'\]\};/, response.body) + assert_match(/const layoutLimits = \{nor8m: \['lite'\]\};/, response.body) end end diff --git a/test/models/camera_test.rb b/test/models/camera_test.rb index a2a069bb..eae0aa6f 100644 --- a/test/models/camera_test.rb +++ b/test/models/camera_test.rb @@ -130,6 +130,61 @@ def camera(flash_type:, firmware_version: 'ultimate') assert_equal '0xD50000', c.overlay_offset end + # --- the chip and the layout are two questions --- + + test 'the layout defaults to the one that matches the chip' do + assert_equal 'nor8m', camera(flash_type: 'nor8m').partition_layout + assert_equal 'nor16m', camera(flash_type: 'nor16m').partition_layout + assert_equal 'nor16m', camera(flash_type: 'nor32m').partition_layout + assert_equal 'nand', camera(flash_type: 'nand').partition_layout + end + + # The configuration the report came from: a 16MB part wearing the 8MB layout, + # which is what a camera flashed from the 8MB image already has. The offsets + # have to be the 8MB ones and the erase has to be the whole chip, because + # rootfs_data is `-` in every mtdparts and so runs to the end of the device + # whichever layout is written on it. + test 'an 8MB layout on a 16MB chip keeps 8MB offsets and a 16MB erase' do + c = camera(flash_type: 'nor16m', firmware_version: 'lite') + c.partition_layout = 'nor8m' + + assert_equal 8, c.layout_size + assert_equal '0x250000', c.rootfs_offset + assert_equal '0x500000', c.rootfs_max_size + assert_equal '0x200000', c.kernel_max_size + assert_equal '0x750000', c.overlay_offset + + assert_equal 16, c.flash_size + assert_equal '0x1000000', c.flash_size_hex + # The whole of the rest of the chip: 0x1000000 - 0x750000, not the 0xb0000 + # an 8MB part would leave. That difference is the untouched half a full + # reflash used to leave the old overlay sitting in. + assert_equal '0x8b0000', c.overlay_max_size + end + + test 'the 16MB layout is refused on an 8MB chip, which cannot hold it' do + c = camera(flash_type: 'nor8m') + c.partition_layout = 'nor16m' + + assert_equal 'nor8m', c.partition_layout + assert_equal '0x250000', c.rootfs_offset + end + + test 'an unrecognised layout is the same as none' do + c = camera(flash_type: 'nor16m') + c.partition_layout = 'nor64m' + + assert_equal 'nor16m', c.partition_layout + end + + test 'nand has one layout and does not take a nor one' do + c = camera(flash_type: 'nand') + c.partition_layout = 'nor8m' + + assert_equal 'nand', c.partition_layout + assert_equal '0x400000', c.rootfs_offset + end + test 'every nor layout leaves the overlay inside the chip' do %w[nor8m nor16m nor32m].each do |flash| %w[lite ultimate].each do |edition| @@ -203,7 +258,15 @@ def camera(flash_type:, firmware_version: 'ultimate') keys = Rack::Utils.parse_query(camera.permalink.delete_prefix('?')).keys - assert_equal %w[mac cip sip net rom ver sd].sort, keys.sort + assert_equal %w[mac cip sip net rom part ver sd].sort, keys.sort + end + + test 'the permanent link carries a layout that is not the chip default' do + camera = Camera.new(camera_mac_address: 'aa:bb:cc:dd:ee:ff', flash_type: 'nor16m', + firmware_version: 'lite', network_interface: 'eth', sd_card_slot: 'nosd') + camera.partition_layout = 'nor8m' + + assert_includes camera.permalink, '&part=nor8m' end test 'the permanent link carries every value it was built from' do @@ -216,7 +279,8 @@ def camera(flash_type:, firmware_version: 'ultimate') # The MAC is the one field that changes shape: colons are not legal in a # query string unescaped, and `show` turns the dashes back. assert_equal({ 'mac' => 'aa-bb-cc-dd-ee-ff', 'cip' => '10.0.0.5', 'sip' => '10.0.0.1', - 'net' => 'wifi', 'rom' => 'nor32m', 'ver' => 'ultimate', 'sd' => 'sd' }, query) + 'net' => 'wifi', 'rom' => 'nor32m', 'part' => 'nor16m', 'ver' => 'ultimate', + 'sd' => 'sd' }, query) end def with_index(assets) diff --git a/test/models/firmware_test.rb b/test/models/firmware_test.rb index 5f9a206a..e9dd54fb 100644 --- a/test/models/firmware_test.rb +++ b/test/models/firmware_test.rb @@ -72,10 +72,10 @@ def uboot_path @uboot_path ||= File.join(@dir, 'u-boot.bin').tap { |p| IO.binwrite(p, UBOOT) } end - def build(model:, vendor:, members:, flash_type:, size:, release: 'ultimate', board: nil) + def build(model:, vendor:, members:, flash_type:, size:, release: 'ultimate', board: nil, layout: nil) soc = StubSoc.new(model: model, vendor: vendor, board: board, uboot_file: uboot_path, linux_file: write_tgz("openipc.#{board || model}-#{flash_type}-#{release}.tgz", members)) - Firmware.new(size: size, flash_type: flash_type, release: release, soc: soc) + Firmware.new(size:, flash_type:, release:, soc:, layout:) end # Half-built images are named for their target, so a leftover is attributable @@ -94,6 +94,28 @@ def leftover_temp_files(firmware) assert_not_equal nor, nand end + # The layout only enters the name when it is not the one the chip would have + # had anyway. Every image already cached was built before the two could + # differ, so the usual combinations have to keep the names they were built + # under or the whole cache is orphaned. + test 'the layout only enters the filename when it is not the chip default' do + natural = Firmware.filename_for(soc_model: 'hi3516ev300', flash_type: 'nor', release: 'lite', + size: 16, layout: 16) + smaller = Firmware.filename_for(soc_model: 'hi3516ev300', flash_type: 'nor', release: 'lite', + size: 16, layout: 8) + + assert_equal 'openipc-hi3516ev300-nor-lite-16mb.bin', natural + assert_equal 'openipc-hi3516ev300-nor-lite-16mb-parts8m.bin', smaller + end + + test 'a filename asked for without a layout is spelled as it always was' do + { 8 => 'openipc-x-nor-lite-8mb.bin', 16 => 'openipc-x-nor-lite-16mb.bin', + 32 => 'openipc-x-nor-lite-32mb.bin' }.each do |size, expected| + assert_equal expected, + Firmware.filename_for(soc_model: 'x', flash_type: 'nor', release: 'lite', size:) + end + end + # --- NOR regression --- test 'nor image keeps its layout' do @@ -108,6 +130,41 @@ def leftover_temp_files(firmware) assert_equal SQUASHFS, image[0x350000, SQUASHFS.bytesize] end + # A 16MB chip carrying the 8MB layout. The parts go where the 8MB mtdparts + # says, and the image is still the size of the chip -- which is what lets the + # installation page erase all of it and leave the old overlay nowhere to + # survive. + test 'a 16MB image can carry the 8MB layout and still fill the chip' do + fw = build(model: 'hi3518ev201', vendor: 'HiSilicon', flash_type: 'nor', size: 16, layout: 8, + members: { 'uImage.hi3518ev201' => KERNEL, 'rootfs.squashfs.hi3518ev201' => SQUASHFS }) + fw.generate + image = IO.binread(fw.filepath) + + assert_equal 16.megabytes, image.bytesize + assert_equal UBOOT, image[0, UBOOT.bytesize] + assert_equal KERNEL, image[0x50000, KERNEL.bytesize] + assert_equal SQUASHFS, image[0x250000, SQUASHFS.bytesize] + # rootfs_data, blank all the way to the end of the chip. + assert_equal ("\xFF".b * 0x100), image[0x750000, 0x100] + assert_equal ("\xFF".b * 0x100), image[16.megabytes - 0x100, 0x100] + end + + # A request parameter, like the size beside it, so it is refused before it + # can decide where anything is written. + test 'a layout larger than the chip is refused' do + fw = build(model: 'hi3518ev202', vendor: 'HiSilicon', flash_type: 'nor', size: 8, layout: 16, + members: { 'uImage.hi3518ev202' => KERNEL, 'rootfs.squashfs.hi3518ev202' => SQUASHFS }) + + assert_raises(Firmware::InvalidFlashSize) { fw.generate } + end + + test 'a layout no bootloader defines is refused' do + fw = build(model: 'hi3518ev203', vendor: 'HiSilicon', flash_type: 'nor', size: 32, layout: 32, + members: { 'uImage.hi3518ev203' => KERNEL, 'rootfs.squashfs.hi3518ev203' => SQUASHFS }) + + assert_raises(Firmware::InvalidFlashSize) { fw.generate } + end + # --- NAND --- test 'nand image takes rootfs.ubi and puts it at the ubi offset' do From e7fff81a5295b0dc07a7dfaf2ea8562cdea417fa Mon Sep 17 00:00:00 2001 From: AI Dev Date: Sat, 29 Aug 2026 06:31:55 +0000 Subject: [PATCH 2/2] Answer the review: three ways the split was still wrong Qodo found three, and driving the wizard script through a DOM stub found a fourth that was mine. **The layout stopped following the chip.** The form opens on nor8m, and nor8m is a legal layout on a larger part, so `checkPartitionLayout` never took it away: picking NOR 16M left 8MB partitions on a 16MB camera and the edition limiter took Ultimate off with them. It now follows the chip until the visitor settles it themselves -- by using the menu, or by arriving on a permanent link that carries a `part` that is not the chip's own, which is as deliberate as clicking. **The rootfs was bounded by the image, not by its partition.** `layout` gave the rootfs part the whole chip-sized image as its limit. That was near enough while the layout was the chip -- an 8MB image ends 0xb0000 past the rootfs partition -- and is not now: a 16MB image laid out the 8MB way ends 0x8b0000 past it, and `download_full_image` takes the edition and the layout straight from the query string. `?fw_release=ultimate&flash_size=16&layout=8` would have written a 7MB rootfs from 0x250000 clean through rootfs_data. It is bounded by the overlay offset now, which is what the mtdparts says the partition is. **"This SoC needs a larger chip" reached chips that are large enough.** The Ultimate guard became the layout's, so the no-Lite branch under it now sees 16MB and 32MB parts, where the advice is not to buy a chip but to choose the other layout. It tells the two apart, like the warning beside it already did. **NAND lost Ultimate.** The layout menu keeps its value while it is hidden, so `checkPartitionLayout` handed back nor8m on a NAND part and the 8MB rootfs rule disabled an edition that eleven of the sixteen NAND boards are published as and nothing else. It answers with no layout where there is none to choose. --- app/controllers/cameras/socs_controller.rb | 23 +++++++++-- app/models/firmware.rb | 23 ++++++++++- app/views/cameras/socs/show.html.erb | 48 +++++++++++++++++++--- test/controllers/socs_controller_test.rb | 43 +++++++++++++++++++ test/models/firmware_test.rb | 25 +++++++++-- 5 files changed, 148 insertions(+), 14 deletions(-) diff --git a/app/controllers/cameras/socs_controller.rb b/app/controllers/cameras/socs_controller.rb index 9567adda..f71afa5e 100644 --- a/app/controllers/cameras/socs_controller.rb +++ b/app/controllers/cameras/socs_controller.rb @@ -342,10 +342,7 @@ def enforce_eight_meg_limit @camera.firmware_version = 'lite' flash.now[:warning] = eight_meg_warning else - flash.now[:alert] = - 'The Ultimate edition does not fit an 8MB flash chip, and OpenIPC publishes no Lite build ' \ - 'for this SoC on NOR. These instructions cannot produce a working camera on 8MB flash -- ' \ - 'this SoC needs a larger chip.' + flash.now[:alert] = no_lite_for_eight_meg_alert end end @@ -355,6 +352,24 @@ def eight_meg_rootfs_with_ultimate? @camera.partition_layout.eql?('nor8m') && @camera.firmware_version.eql?('ultimate') end + # "This SoC needs a larger chip" is the right advice for an 8MB part and the + # wrong advice for a 16MB one wearing the 8MB layout, where the chip is + # already big enough and the layout is the thing to change. The guard above + # reaches both since it became the layout's, so this has to tell them apart + # too -- it is the branch for a SoC published as Ultimate and nothing else, + # hi3516cv6xx and hi3519dv500, where there is no Lite to fall back to. + def no_lite_for_eight_meg_alert + unless @camera.flash_type.eql?('nor8m') + return 'The Ultimate edition does not fit the 8MB partition layout, and OpenIPC publishes ' \ + 'no Lite build for this SoC on NOR. Choose the 16MB layout, which this chip is big ' \ + 'enough for.' + end + + 'The Ultimate edition does not fit an 8MB flash chip, and OpenIPC publishes no Lite build ' \ + 'for this SoC on NOR. These instructions cannot produce a working camera on 8MB flash -- ' \ + 'this SoC needs a larger chip.' + end + # The chip when the chip is what limits them, and the layout when it is the # layout: a 5120KB rootfs partition is a 5120KB rootfs partition whether the # part around it is 8MB or 32MB, and on the larger ones there is something diff --git a/app/models/firmware.rb b/app/models/firmware.rb index 60b2b4a4..a0e586a4 100644 --- a/app/models/firmware.rb +++ b/app/models/firmware.rb @@ -233,10 +233,31 @@ def layout(uboot, kernel, rootfs, size) [ Part.new('u-boot', uboot, 0, kernel_offset, 'the kernel offset'), Part.new('kernel', kernel, kernel_offset, rootfs_offset, 'the rootfs offset'), - Part.new('rootfs', rootfs, rootfs_offset, size, 'the end of the image') + Part.new('rootfs', rootfs, rootfs_offset, *rootfs_limit(size)) ] end + # The rootfs may not run past its own partition, which on NOR is where the + # overlay starts and not the end of the image. + # + # The two were near enough the same number while the layout was the chip: an + # 8MB image laid out the 8MB way ends 0xb0000 past the rootfs partition, and + # the difference only mattered for a rootfs already too big for the partition + # to mount. A 16MB image laid out the 8MB way ends 0x8b0000 past it, and + # download_full_image takes the edition and the layout straight from the + # query string -- so `?fw_release=ultimate&flash_size=16&layout=8` would have + # written a 7MB rootfs from 0x250000 clean through rootfs_data, and the + # camera would have mounted a squashfs whose tail the overlay then formatted + # over. + # + # NAND keeps the end of the image, which is where its rootfs ends by + # construction: image_size is the rootfs offset plus the payload. + def rootfs_limit(size) + return [size, 'the end of the image'] if nand? + + [nor_layout[:overlay_offset], 'the rootfs partition'] + end + # Build beside the destination and rename into place. # # This used to build in Dir.tmpdir and hand the result to FileUtils.mv. In diff --git a/app/views/cameras/socs/show.html.erb b/app/views/cameras/socs/show.html.erb index 21a1d5d4..797b9d40 100644 --- a/app/views/cameras/socs/show.html.erb +++ b/app/views/cameras/socs/show.html.erb @@ -145,10 +145,25 @@ return chip === 'nor8m' ? ['nor8m'] : ['nor8m', 'nor16m']; } + // The layout a chip wears unless it is told otherwise: the largest one it + // can hold, which is the last entry allowedLayouts returns. + function naturalLayout(chip) { + const allowed = allowedLayouts(chip); + return allowed.length ? allowed[allowed.length - 1] : ''; + } + + // Whether the visitor has settled the layout for themselves. Until they + // have, it follows the chip, so choosing NOR 16M gets the 16MB layout -- + // which is what the single menu this replaced always produced. + // + // Without it the 8MB layout the form opens on survived every later chip + // change, because it is a legal choice on a larger part and so never + // tripped the not-allowed branch. Picking NOR 16M then quietly gave a 16MB + // camera 8MB partitions, and the edition limiter below took Ultimate away + // with it. + let layoutChosen = false; + // Narrow the layout menu to the chip, and answer what is now selected. - // Landing on the last allowed entry rather than the first makes the - // chip's own layout the default, which is what this form produced when - // the two were a single field. function checkPartitionLayout() { const el = document.querySelector('#camera_partition_layout'); const chip = document.querySelector('#camera_flash_type').value; @@ -159,9 +174,17 @@ const o = el.options.item(i); o.disabled = !allowed.includes(o.value); } - if (allowed.length && !allowed.includes(el.value)) el.value = allowed[allowed.length - 1]; + if (allowed.length && !(layoutChosen && allowed.includes(el.value))) { + el.value = naturalLayout(chip); + } - return el.value; + // Nothing, rather than whatever the hidden menu happens to hold, when + // the chip has no NOR layout to choose. The menu keeps its value while + // it is hidden, so NAND was reading back `nor8m` and having Ultimate + // taken off it by a rootfs partition it does not have -- eleven of the + // sixteen boards with a NAND build are published as Ultimate and + // nothing else. + return allowed.length ? el.value : ''; } function allowedEditions(chip, layout) { @@ -232,6 +255,19 @@ document.querySelector('#generate-mac-address').addEventListener('click', generateMacAddress); document.querySelector('#camera_flash_type').addEventListener('change', checkFlashSize); - document.querySelector('#camera_partition_layout').addEventListener('change', checkFlashSize); + document.querySelector('#camera_partition_layout').addEventListener('change', function () { + layoutChosen = true; + checkFlashSize(); + }); + + // Settled before the first narrowing, and after the chip has: a page that + // opens on a layout that is not its chip's own got there from a permanent + // link carrying `part`, and that is as deliberate as using the menu. A link + // written before this field existed carries none, so it opens on the chip's + // own layout and means exactly what it always did. + useAnOfferedFlashType(); + layoutChosen = allowedLayouts(document.querySelector('#camera_flash_type').value).length > 0 && + document.querySelector('#camera_partition_layout').value + !== naturalLayout(document.querySelector('#camera_flash_type').value); checkFlashSize(); diff --git a/test/controllers/socs_controller_test.rb b/test/controllers/socs_controller_test.rb index f63dce34..d93bb1ca 100644 --- a/test/controllers/socs_controller_test.rb +++ b/test/controllers/socs_controller_test.rb @@ -272,6 +272,49 @@ def submit(soc, flash_type, firmware_version: 'lite', partition_layout: nil, loc end end + # The other half of that rule, for a SoC with no Lite build to fall back to. + # "This SoC needs a larger chip" is right for an 8MB part and wrong here: the + # chip is 16MB and it is the layout on it that Ultimate does not fit. + test 'a SoC published only as Ultimate is told to change the layout, not the chip' do + soc = instructable_soc('TS3516EVE50') + + with_release_index("openipc.#{soc.board}-nor-ultimate.tgz") do + submit(soc, 'nor16m', firmware_version: 'ultimate', partition_layout: 'nor8m') + + assert_match 'Choose the 16MB layout, which this chip is big enough for', response.body + assert_no_match(/needs a larger chip/, response.body) + end + end + + test 'an 8MB chip with no Lite build is still told it needs a larger one' do + soc = instructable_soc('TS3516EVE60') + + with_release_index("openipc.#{soc.board}-nor-ultimate.tgz") do + submit(soc, 'nor8m', firmware_version: 'ultimate') + + assert_match 'needs a larger chip', response.body + end + end + + # The layout menu follows the chip until the visitor settles it themselves, + # so picking a larger chip does not leave the 8MB layout the form opens on + # sitting on it -- which would quietly hand a 16MB camera 8MB partitions and + # take Ultimate away with them. + test 'the layout menu follows the chip until the visitor picks one' do + soc = instructable_soc('TS3516EVE70') + + with_release_index(*every_edition_for(soc)) do + get "/cameras/vendors/#{@vendor.to_param}/socs/#{soc.to_param}" + + assert_response :success + assert_match(/el.value = naturalLayout\(chip\);/, response.body) + assert_match(/layoutChosen = true;/, response.body) + # ...and answers with no layout at all where there is none to choose, so + # a hidden menu still holding nor8m cannot take Ultimate off a NAND part. + assert_match(/return allowed.length \? el.value : '';/, response.body) + end + end + test 'the permanent link carries the layout so it can be reopened' do soc = instructable_soc('TS3516EVE40') diff --git a/test/models/firmware_test.rb b/test/models/firmware_test.rb index e9dd54fb..c406988d 100644 --- a/test/models/firmware_test.rb +++ b/test/models/firmware_test.rb @@ -313,7 +313,7 @@ def leftover_temp_files(firmware) # --- a part too big for its slot --- - test 'a rootfs too large for the flash is refused rather than written past the end' do + test 'a rootfs too large for its partition is refused rather than written past the end' do # openipc-hi3516ev200-nor-ultimate-8mb.bin was found in the production # cache at 9,465,856 bytes -- 0x250000 plus a 7MB Ultimate rootfs, in a # file whose name promises 8MB. IO.binwrite past the end grows the file @@ -327,8 +327,12 @@ def leftover_temp_files(firmware) assert_not File.exist?(fw.filepath), 'an image larger than its flash must not be left to be served' end - test 'a rootfs that exactly fills the flash is still built' do - exact = "\xC3".b * (8.megabytes - 0x250000) + test 'a rootfs that exactly fills its partition is still built' do + # 0x250000 to 0x750000 -- the 5120KB the 8MB mtdparts gives the rootfs, not + # the 0x5b0000 that is left before the end of an 8MB image. The last + # 0xb0000 of the chip is rootfs_data, and a squashfs written into it is one + # the kernel cannot mount anyway: root is mtdblock3, which is 5120KB long. + exact = "\xC3".b * (0x750000 - 0x250000) fw = build(model: 'hi3516ev200', vendor: 'HiSilicon', flash_type: 'nor', size: 8, members: { 'uImage.hi3516ev200' => KERNEL, 'rootfs.squashfs.hi3516ev200' => exact }) fw.generate @@ -336,6 +340,21 @@ def leftover_temp_files(firmware) assert_equal 8.megabytes, File.size(fw.filepath) end + # Room in the image is not room in the partition. This one fits a 16MB chip + # twice over and does not fit the 8MB layout that was asked for with it -- + # and download_full_image takes both straight from the query string, so + # nothing upstream of here refuses the combination. + test 'a rootfs that fits the chip but not its partition is refused' do + oversize = "\xC3".b * (0x750000 - 0x250000 + 1) + fw = build(model: 'hi3516ev204', vendor: 'HiSilicon', flash_type: 'nor', size: 16, layout: 8, + members: { 'uImage.hi3516ev204' => KERNEL, 'rootfs.squashfs.hi3516ev204' => oversize }) + + error = assert_raises(Firmware::PayloadTooLarge) { fw.generate } + assert_match(/rootfs/, error.message) + assert_match(/the rootfs partition/, error.message) + assert_not File.exist?(fw.filepath) + end + test 'a kernel that would run into the rootfs is refused' do oversize = "\xA5".b * (0x250000 - 0x50000 + 1) fw = build(model: 'hi3516ev200', vendor: 'HiSilicon', flash_type: 'nor', size: 8,