First automation beginner

Hi i have been back and forth from HA to HB and really want to make the jump if i can just get this first automation to work i can duplicate it for everything else.
So simply i want my motion sensor to turn my lights on 30 mins before sunset for 1 minute then turn them off and the automation to stop at sunrise. i have tried asking on fb but get a lot of different answers ( use the or condition or use the below horizon one) but it seems way over my head.
So made up this automation and it seems to work for the last week, is there anything wrong please ?
Thanks

In the future, please paste your automation to the post with proper formatting so we do not have to re-type it.

As you have it currently, your conditions are not limiting your automation in any way… it will execute every time motion is detected. To explain:

before: sunset
before_offset: '00:30:00'

The above clause makes the condition true from just after midnight until 30 minutes past sunset. This effectively makes the sunrise condition pointless.

after: sunset

The above clause makes the condition true from sunset until midnight.

The combined effect is that it is true from midnight until the next midnight…

To limit your automation so that it will only execute in the hours 30 minutes after sunset until sunrise you use the following condition:

condition:
  condition: or
  conditions:
    - condition: sun
      after: sunset
      after_offset: '00:30:00'
    - condition: sun
      before: sunrise

Thanks, but whenever i change anything in the first group of conditions it fails to run

alias: Turn shower light on when motion is detected at night
description: ''
trigger:
  - type: motion
    platform: device
    device_id: 2b5335e9b654c6b0be7f568b08ea836c
    entity_id: binary_sensor.shower_motion_sensor_3
    domain: binary_sensor
condition:
  - condition: or
    conditions:
      - condition: sun
        before: sunrise
      - condition: sun
        after: sunset
action:
  - service: light.turn_on
    data: {}
    target:
      entity_id: light.shower_lights_2
  - delay:
      hours: 0
      minutes: 5
      seconds: 0
      milliseconds: 0
  - service: light.turn_off
    data: {}
    target:
      entity_id: light.shower_lights_2
mode: restart

In your code you have different conditions as an ‘or’. Re-write them as a single condition as shown in the previous answer.

Also in your trigger you might want to change it from motion to state. Then write the trigger to fire when the state of the motion sensor changes to ‘on’.

Actually don’t do this.

It is not possible to be before sunrise AND after sunset on the same day. See:

https://www.home-assistant.io/docs/scripts/conditions/#sunsetsunrise-condition

Give your automation an id: so you can use the automation trace feature to see why it is not running as you expect:

Sorry, my mistake. There is another system I moved from where this was possible.

Using the automation trace feature really has been key for me learning how to get automations reliably working.